# Shell Script Security Audit

**Date:** 2026-02-20
**Auditor:** External Security Review
**Scope:** All shell scripts in project root and `scripts/` directory

## Executive Summary

| Metric | Value |
|--------|-------|
| Total Scripts Analyzed | 45+ |
| Scripts Missing `set -e` | 38 |
| Scripts Missing `set -u` | 45 (all) |
| Scripts with Unquoted Variables | 4 |

## Detailed Findings

### 1. Missing Error Handling (HIGH)

**Severity:** HIGH
**Affected:** 38 of 45 scripts (84%)
**CWE:** CWE-252 (Unchecked Return Value)

#### Description
Most shell scripts lack `set -e` which causes scripts to continue executing even when commands fail. This can lead to:
- Silent failures going undetected
- Partial/incomplete operations
- Security bypasses if validation commands fail

#### Pattern Found
```bash
#!/bin/bash
# Script continues even if commands fail
docker build ...  # If this fails, script continues
echo "Build complete"  # Misleading success message
```

#### Correct Pattern
```bash
#!/bin/bash
set -euo pipefail
# Script exits immediately on any error
```

#### Affected Script Categories
- **Wrapper scripts** (mcp-*-wrapper.sh): 34 scripts
- **LSP wrapper scripts** (lsp-*-wrapper.sh): Multiple
- **Build scripts** (scripts/BuildAll.sh): 1 script

#### Recommendation
Add to all scripts:
```bash
#!/bin/bash
set -euo pipefail
```

---

### 2. Missing Undefined Variable Protection (HIGH)

**Severity:** HIGH
**Affected:** 45 of 45 scripts (100%)
**CWE:** CWE-456 (Missing Initialization of a Variable)

#### Description
No scripts use `set -u` which prevents the use of undefined variables. This can cause:
- Commands to execute with empty values
- Unexpected behavior with missing environment variables
- Potential security issues if variables like paths are empty

#### Example Risk
```bash
#!/bin/bash
# If API_KEY is not set, this becomes: curl -H "Authorization: Bearer "
curl -H "Authorization: Bearer $API_KEY" https://api.example.com
```

#### Recommendation
```bash
#!/bin/bash
set -u  # Exit on undefined variable

# Or provide defaults:
API_KEY="${API_KEY:-}"  # Empty default
API_KEY="${API_KEY:-default_value}"  # Non-empty default
```

---

### 3. Unquoted Variables (MEDIUM)

**Severity:** MEDIUM
**Affected:** 4 scripts
**CWE:** CWE-77 (Command Injection)

#### Description
Some scripts use unquoted variables which can lead to:
- Word splitting on spaces
- Glob expansion
- Potential command injection

#### Affected Files
Found in various wrapper scripts where environment variables are passed through.

#### Pattern to Avoid
```bash
# Dangerous - unquoted
docker run $CONTAINER_NAME

# Safe - quoted
docker run "$CONTAINER_NAME"
```

#### Recommendation
Always quote variable expansions:
```bash
docker run "${CONTAINER_NAME}"
```

---

### 4. No Input Validation (MEDIUM)

**Severity:** MEDIUM
**Affected:** All wrapper scripts
**CWE:** CWE-20 (Improper Input Validation)

#### Description
Wrapper scripts pass environment variables directly to Docker commands without validation.

#### Example
```bash
#!/bin/bash
docker run --rm -i \
  -e PROXMOX_HOST="${PROXMOX_HOST}" \
  -e PROXMOX_USER="${PROXMOX_USER}" \
  kneldevstack-aimiddleware-proxmox-mcp
```

No validation that:
- Variables are set
- Values are in expected format
- Values don't contain injection characters

#### Recommendation
Add validation for critical variables:
```bash
#!/bin/bash
set -euo pipefail

: "${REQUIRED_VAR:?REQUIRED_VAR must be set}"

# Optional validation
if [[ ! "$URL" =~ ^https?:// ]]; then
  echo "Invalid URL format" >&2
  exit 1
fi
```

---

## Script Categories

### MCP Wrapper Scripts (34 files)
- Pattern: `mcp-*-wrapper.sh`
- Purpose: Launch MCP server containers with environment configuration
- Risk: Medium - primarily pass-through scripts

### LSP Wrapper Scripts (5+ files)
- Pattern: `lsp-*-wrapper.sh`
- Purpose: Launch LSP server containers
- Risk: Low - typically no sensitive data

### Build/Utility Scripts (scripts/)
- `BuildAll.sh` - Build automation
- `validate-all.sh` - Validation testing
- Risk: Low - development utilities

---

## Positive Findings

1. **No command substitution injection** - Variables passed as arguments, not evaluated
2. **Consistent naming convention** - Easy to identify script purpose
3. **Simple pass-through design** - Limited attack surface
4. **No hardcoded secrets** - All credentials from environment

---

## Remediation Priority

| Priority | Finding | Effort | Impact |
|----------|---------|--------|--------|
| 1 | Add `set -euo pipefail` to all scripts | Low | High |
| 2 | Quote all variable expansions | Low | Medium |
| 3 | Add input validation for critical vars | Medium | Medium |

---

## Example Secure Script Template

```bash
#!/bin/bash
# MCP Server Wrapper for: service-name
# Generated secure template

set -euo pipefail

# Required variables with validation
: "${API_URL:?API_URL must be set}"
: "${API_KEY:?API_KEY must be set}"

# Optional variables with defaults
TIMEOUT="${TIMEOUT:-30}"
DEBUG="${DEBUG:-false}"

# Input validation
if [[ ! "$API_URL" =~ ^https?:// ]]; then
  echo "ERROR: API_URL must be a valid HTTP/HTTPS URL" >&2
  exit 1
fi

# Execute
exec docker run --rm -i \
  -e API_URL="${API_URL}" \
  -e API_KEY="${API_KEY}" \
  -e TIMEOUT="${TIMEOUT}" \
  -e DEBUG="${DEBUG}" \
  kneldevstack-aimiddleware-service-name
```