Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ If you do all 8: you have layered defenses against D-1 / D-2 / D-3 / D-4 / D-7 u
| **D-2** | Security | `/api/forge` fetches arbitrary URL → LLM → writes skill, no review gate | **W1 — CLOSED ([#43](https://github.com/AVADSA25/codec/pull/43), `ff16664`)** |
| **D-3** | Security | `/api/save_skill` writes directly to skills/ with only substring check | **W1 — CLOSED ([#43](https://github.com/AVADSA25/codec/pull/43), `ff16664`)** |
| **D-4** | Security | `file_write` skill (MCP-exposed) can write to `~/.codec/skills/` | **W1 — CLOSED ([#45](https://github.com/AVADSA25/codec/pull/45), `0065d90`)** |
| **D-5** | Security | `permission_gate` accepts path-traversal via `fnmatch` (no realpath) | **W1 — CLOSED (PR-1D)** |
| **D-5** | Security | `permission_gate` accepts path-traversal via `fnmatch` (no realpath) | **W1 — CLOSED ([#47](https://github.com/AVADSA25/codec/pull/47), `fd2b460`)** |
| C-1 | Reliability | `codec.py` daemon ignores SIGINT/SIGTERM; leaks sox + tkinter on every restart | W4 |
| C-2 | Reliability | `~/.codec/pwa_response.json` race conditions + no correlation_id | W4 |
| C-3 | Reliability | `notifications.json` has 3 writers with 3 different write semantics | W4 |
Expand Down
6 changes: 3 additions & 3 deletions docs/audits/PHASE-1-SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ The skill is `SKILL_MCP_EXPOSE = True` and NOT in `_HTTP_BLOCKED` (`codec_config
**Location:** `codec_agent_runner.py:95-142` (`permission_gate`)
**CWE / OWASP:** CWE-22 Path Traversal / CWE-23 Relative Path Traversal / Agentic A06 Excessive Agency

> **Closed by PR-1D** (branch `fix/pr1d-permission-gate-realpath`). `permission_gate` was refactored: (1) reject `..` segments outright before any matching, (2) `os.path.realpath` both sides of every comparison, (3) `fnmatch` replaced with `action_real.startswith(grant_real + os.sep)`. New helper `_path_allowed` consolidates the logic for both `touches_path` and `reads_path`. Rejections emit `permission_gate_blocked` audit events (source=`codec-agent-runner`, outcome=`error`, level=`warning`, extra={`requested_path`, `resolved_path`, `reason`, `agent_id`}) before raising `PermissionViolation`. 5 new tests in `tests/test_agent_runner.py` cover dotdot rejection (write + read paths), symlink-outside-grant rejection, symlink-within-grant acceptance, and audit emission. All 7 pre-existing permission_gate tests still pass.
> **Closed by PR-1D** ([#47](https://github.com/AVADSA25/codec/pull/47), merged as `fd2b460`). `permission_gate` was refactored: (1) reject `..` segments outright before any matching, (2) `os.path.realpath` both sides of every comparison, (3) `fnmatch` replaced with `action_real.startswith(grant_real + os.sep)`. New helper `_path_allowed` consolidates the logic for both `touches_path` and `reads_path`. Rejections emit `permission_gate_blocked` audit events (source=`codec-agent-runner`, outcome=`error`, level=`warning`, extra={`requested_path`, `resolved_path`, `reason`, `agent_id`}) before raising `PermissionViolation`. 5 new tests in `tests/test_agent_runner.py` cover dotdot rejection (write + read paths), symlink-outside-grant rejection, symlink-within-grant acceptance, and audit emission. All 7 pre-existing permission_gate tests still pass.
**Description:** The Step 9 permission gate checks `action.path` via `fnmatch.fnmatch(os.path.expanduser(action.path), os.path.expanduser(p))`. `expanduser` resolves `~`, but does NOT resolve `..` or symlinks. `fnmatch` is glob-style and does NOT treat `..` specially. So:
- Plan grant: `~/Documents/**`
- Action path: `~/Documents/../../etc/passwd`
Expand Down Expand Up @@ -271,7 +271,7 @@ The skill is `SKILL_MCP_EXPOSE = True` and not in `_HTTP_BLOCKED`, so an attacke
**Location:** `codec_agent_plan.py:754-759`
**CWE / OWASP:** CWE-20 Improper Input Validation / Agentic A06 Excessive Agency

> **Closed by PR-1D** (branch `fix/pr1d-permission-gate-realpath`). `_PATH_BLOCKLIST_SUBSTRINGS` extended with 8 new entries: `/.codec/skills`, `/.codec/plugins`, `/.codec/oauth_state.json`, `/.codec/audit.log`, `/.codec/agents`, `/.codec/agent_global_grants.json`, `/.codec/config.json`, `/.codec/memory.db`. The LLM-drafted plan auto-extract path now drops any user-typed path landing in these directories — chain to D-1 RCE via auto-granted plan write_paths is closed. 7 parametrized tests in `tests/test_agent_plan.py` cover each new entry.
> **Closed by PR-1D** ([#47](https://github.com/AVADSA25/codec/pull/47), merged as `fd2b460`). `_PATH_BLOCKLIST_SUBSTRINGS` extended with 8 new entries: `/.codec/skills`, `/.codec/plugins`, `/.codec/oauth_state.json`, `/.codec/audit.log`, `/.codec/agents`, `/.codec/agent_global_grants.json`, `/.codec/config.json`, `/.codec/memory.db`. The LLM-drafted plan auto-extract path now drops any user-typed path landing in these directories — chain to D-1 RCE via auto-granted plan write_paths is closed. 7 parametrized tests in `tests/test_agent_plan.py` cover each new entry.
**Description:** When the user types a path in their project description (e.g. "save outputs to ~/Documents/foo"), `extract_user_paths` auto-grants the path in the manifest. The blocklist excludes `/.ssh`, `/.aws`, `/Library/Keychains`, `/etc/`, `/var/`, etc., AND `/.codec/secrets`, `/.codec/auth`. But it does NOT exclude `/.codec/skills`, `/.codec/plugins`, `/.codec/oauth_state.json`, or the repo's `skills/` directory. An LLM-drafted plan based on a user's vague description that mentions `~/.codec/skills/util/` would auto-grant write to that path → next agent run can drop a skill → D-1.
**Exploit chain:** User says "build me a skill that does X, save it to ~/.codec/skills/x_skill.py". Plan auto-grants `~/.codec/skills/x_skill.py/**`. Agent writes payload. Restart → RCE.
**Impact:** Bypass of the skill review gate via the auto-extract path.
Expand Down Expand Up @@ -306,7 +306,7 @@ Bonus: `auth_pin_hash` uses unsalted SHA-256. A 4-6-digit PIN is rainbow-table'd
**Location:** `codec_agent_plan.py:791`
**CWE / OWASP:** CWE-20 Improper Input Validation

> **Closed by PR-1D** (branch `fix/pr1d-permission-gate-realpath`). Replaced `any(b in raw for b in _PATH_BLOCKLIST_SUBSTRINGS)` with `_is_path_blocklisted(raw)` which calls `_path_segments_match` on each entry. `_path_segments_match` checks whether the blocklist pattern's `/`-separated segments appear as a CONSECUTIVE SUBSEQUENCE of the path's segments (path is `expanduser` + `normpath`-ed first to collapse `..` and `.`). So `~/Documents/notes_ssh/foo.md` no longer matches `/.ssh` as a false positive (the segment is `notes_ssh`, not `.ssh`), but `~/.ssh/config` still does. Three regression-coverage tests in `tests/test_agent_plan.py` (segment-aware false-positive negative, still-blocks-real-ssh, legitimate-user-paths-pass-through).
> **Closed by PR-1D** ([#47](https://github.com/AVADSA25/codec/pull/47), merged as `fd2b460`). Replaced `any(b in raw for b in _PATH_BLOCKLIST_SUBSTRINGS)` with `_is_path_blocklisted(raw)` which calls `_path_segments_match` on each entry. `_path_segments_match` checks whether the blocklist pattern's `/`-separated segments appear as a CONSECUTIVE SUBSEQUENCE of the path's segments (path is `expanduser` + `normpath`-ed first to collapse `..` and `.`). So `~/Documents/notes_ssh/foo.md` no longer matches `/.ssh` as a false positive (the segment is `notes_ssh`, not `.ssh`), but `~/.ssh/config` still does. Three regression-coverage tests in `tests/test_agent_plan.py` (segment-aware false-positive negative, still-blocks-real-ssh, legitimate-user-paths-pass-through).
**Description:** `if any(b in raw for b in _PATH_BLOCKLIST_SUBSTRINGS): continue` — the substring `/.ssh` is checked against the raw path. But variations like `~/Documents/.ssh-backup/` would match (contains `/.ssh`), correctly blocking. However, `~/Documents/notes_ssh/` does NOT contain `/.ssh` (no leading dot+s before), so it passes — which is correct, but it shows the rule is purely substring. More worryingly, `~/Documents/passthrough/` doesn't contain any blocklist substring even though `passthrough` semantically signals "this is sensitive" (just an example — but the more important miss is `/.codec/skills` from D-14).
**Impact:** Sub-finding of D-14. False sense of security from a fragile substring filter.
**Recommended fix:** Convert to a regex-anchored or path-segment-aware check. Use `Path(raw).parts` and check for any forbidden segment.
Expand Down
Loading