passepartout/docs/rca/rca-shell-hardening.org

Root Cause Analysis: Shell Actuator Security Hardening

• Executive Summary
• 1. Issue: Command Injection Vulnerability
  • Symptoms
  • Root Cause
  • Resolution
• 2. Side-Issue: Missing Package Context
  • Symptoms
  • Root Cause
  • Resolution
• 3. org-agent Mandate Alignment
  • Invariant Check
• 4. Permanent Learnings

Executive Summary

During the formal verification of the `org-skill-shell-actuator`, a critical command injection vulnerability was identified and patched. The previous implementation relied on a naive whitelist check that could be bypassed using shell metacharacters.

1. Issue: Command Injection Vulnerability

Symptoms

Commands like `ls ; rm -rf /` were potentially executable if the first word (`ls`) was in the whitelist.

Root Cause

The `execute-shell-safely` function only checked the first space-delimited word of the command string against the `*allowed-commands*` whitelist. Since `uiop:run-program` executes string-based commands via `/bin/sh -c`, the shell would process the entire string, including injected commands following metacharacters like `;`, `&`, or `|`.

Resolution

1. Metacharacter Blacklist: Introduced `*shell-metacharacters*` containing dangerous shell symbols (`; & | > < $ \` \ !`).
2. Strict Validation: Updated `execute-shell-safely` to scan the entire command string for these characters before performing the whitelist check.
3. Defense-in-Depth: Any command containing a metacharacter is now rejected with a "Security Violation" error, even if the primary command is whitelisted.

2. Side-Issue: Missing Package Context

Symptoms

`UNDEFINED-FUNCTION EXECUTE-SHELL-SAFELY` during unit tests.

Root Cause

`src/shell-logic.lisp` was missing an `(in-package :org-agent)` declaration, causing symbols to be defined in the default `COMMON-LISP-USER` package instead of the harness package.

Resolution

Added the `in-package` header to `shell-logic.lisp`.

3. org-agent Mandate Alignment

Invariant Check

• High-Integrity Memory: The shell actuator is now formally verified with 4 new unit tests covering whitelist enforcement and injection blocking.
• Literate Programming: Updated `org-skill-shell-actuator.org` Phase A and Build sections to reflect the hardened logic.

4. Permanent Learnings

• Whole-String Validation: Never assume that whitelisting the "head" of a command string is sufficient when passing that string to a shell.
• Subshell Avoidance: While the current fix blacklists metacharacters, future iterations should move toward passing command arguments as a Lisp list to `uiop:run-program`, bypassing the shell entirely.