docs(audit): add comprehensive security audit report
External security audit of KNEL-AIMiddleware before release: - FINAL-REPORT.md: Executive summary, risk assessment, remediation roadmap - 01-dockerfile-security.md: 38/40 containers run as root (HIGH) - 02-shell-script-security.md: 83 missing set -e/u directives (HIGH) - 03-docker-compose-security.md: 3 privileged services documented (MEDIUM) - 04-secrets-audit.md: PASS - no hardcoded secrets found - 05-vulnerability-scan.md: 14+ CVEs, 1 CRITICAL OpenSSL (golang:1.23-alpine) Assessment: CONDITIONAL PASS for release 💘 Generated with Crush Assisted-by: GLM-5 via Crush <crush@charm.land>
This commit is contained in:
Charles N Wyble
230
docs/audit/2026-02-20/02-shell-script-security.md
Normal file
230
docs/audit/2026-02-20/02-shell-script-security.md
Normal file
@@ -0,0 +1,230 @@
|
||||
# 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
|
||||
```
|
||||
Reference in New Issue
Block a user