Skip to content

fix(security): D-5 + D-14 + D-16 — permission_gate realpath + blocklist segment-aware#47

Merged
AVADSA25 merged 1 commit into
mainfrom
fix/pr1d-permission-gate-realpath
May 17, 2026
Merged

fix(security): D-5 + D-14 + D-16 — permission_gate realpath + blocklist segment-aware#47
AVADSA25 merged 1 commit into
mainfrom
fix/pr1d-permission-gate-realpath

Conversation

@AVADSA25
Copy link
Copy Markdown
Owner

What

Phase 1 Wave 1 — PR-1D. The final Wave 1 PR. Closes three findings in one cohesive change to the Step 9 agent runtime:

Finding Severity Title
D-5 CRITICAL permission_gate accepts path-traversal via fnmatch
D-14 MEDIUM _PATH_BLOCKLIST_SUBSTRINGS misses ~/.codec/skills and ~/.codec/plugins
D-16 MEDIUM blocklist substring match is anchorless

All three are auto-extract / runtime paths an LLM-drafted plan could use to chain into the D-1 RCE. Each layer closes independently.

How

codec_agent_runner.pypermission_gate refactor (D-5)

Before: fnmatch.fnmatch(os.path.expanduser(action.path), os.path.expanduser(grant)). expanduser resolves ~ but NOT .. or symlinks. So ~/Documents/../../etc/passwd glob-matched ~/Documents/** → action allowed.

After: new helper _path_allowed(action_path, grants) → (allowed, reason):

  1. Reject .. segments outright (if ".." in Path(expanded).parts: return False, "path_traversal")
  2. os.path.realpath the action — symlinks resolved
  3. For each grant, strip glob suffix → os.path.realpath the directory root → action_real.startswith(grant_real + os.sep)

Trade-off: a grant like ~/Documents/*.md now accepts any file under realpath(~/Documents/), not just *.md. Safety > granularity per the audit's explicit recommendation.

Plus: new helper _emit_gate_blocked emits permission_gate_blocked audit event (source=codec-agent-runner, outcome=error, level=warning, extra={requested_path, resolved_path, reason, agent_id}) before raising PermissionViolation. Complementary to the existing agent_blocked_on_permission event from _run_agent.

codec_agent_plan.py — blocklist extension + segment-aware (D-14 + D-16)

Extended _PATH_BLOCKLIST_SUBSTRINGS 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.

Segment-aware matching (D-16): new helper _path_segments_match(path, pattern) checks whether the pattern's /-separated segments appear as a CONSECUTIVE SUBSEQUENCE of the path's segments. path is expanduser + normpath-ed first to collapse .. and ..

Before (substring) After (segment-aware)
~/Documents/notes_ssh/ matches /.ssh ❌ false positive NO match (segment is notes_ssh, not .ssh) ✅
~/.ssh/config matches /.ssh Still matches (segment is .ssh) ✅
/etc/passwd matches /etc/ Still matches ✅
/Library/Application Support/com.apple/x matches Still matches (3-segment sequence) ✅

Tests

+15 new tests (TDD: RED → GREEN):

tests/test_agent_runner.py (+5):

  • test_permission_gate_rejects_path_traversal_dotdot
  • test_permission_gate_rejects_read_path_traversal
  • test_permission_gate_rejects_symlink_outside_grant (real symlink in tmp_path)
  • test_permission_gate_accepts_realpath_within_grant
  • test_permission_gate_emits_blocked_audit_event

tests/test_agent_plan.py (+10):

  • 7 × test_extract_user_paths_blocks_sensitive_codec_dirs (parametrized over the 7 new sensitive paths)
  • test_extract_user_paths_segment_aware_not_false_positive
  • test_extract_user_paths_still_blocks_real_ssh_path
  • test_extract_user_paths_allows_legitimate_user_paths

Sample audit emission

{"event": "permission_gate_blocked",
 "source": "codec-agent-runner",
 "outcome": "error",
 "level": "warning",
 "message": "permission_gate refused '~/Documents/../../etc/passwd': path_traversal",
 "extra": {"requested_path": "~/Documents/../../etc/passwd",
            "resolved_path": "/etc/passwd",
            "reason": "path_traversal",
            "agent_id": ""}}

Files

File Change
codec_agent_runner.py Refactor permission_gate, new _path_allowed + _emit_gate_blocked helpers; removed unused fnmatch import. +96/−9.
codec_agent_plan.py Extended _PATH_BLOCKLIST_SUBSTRINGS (+8 entries), new _path_segments_match + _is_path_blocklisted helpers. +55/−2.
tests/test_agent_runner.py +5 permission_gate tests. +143 LOC.
tests/test_agent_plan.py +10 extract_user_paths tests. +80 LOC.
AGENTS.md §7 New "Agent permission gate + path blocklist (PR-1D)" subsection.
docs/audits/PHASE-1-SECURITY.md D-5 + D-14 + D-16 closure footnotes.
docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md D-5 row flipped to W1 — CLOSED (PR-1D).

Diff: 7 files, +387/−12.

Verification

Check Result
pytest tests/test_agent_runner.py tests/test_agent_plan.py 12 + 52 = 64 passed (15 new + 49 pre-existing)
Full Wave 1 regression (5 test files) 148 passed
python3 tests/test_skill_imports.py 76 parsed, 0 errors
python3 tools/generate_skill_manifest.py --check ok
ruff check codec_agent_runner.py codec_agent_plan.py Only pre-existing errors (verified by stash-and-rerun)

Wave 1 status after this PR merges

Finding Severity Closing PR
D-1 CRITICAL PR-1A (#42, 48ec5d5) ✅
D-2 CRITICAL PR-1B (#43, ff16664) ✅
D-3 CRITICAL PR-1B (#43, ff16664) ✅
D-4 CRITICAL PR-1C (#45, 0065d90) ✅
D-5 CRITICAL This PR
D-14 MEDIUM (bonus) This PR
D-16 MEDIUM (bonus) This PR

All 5 CRITICAL skill-loading + write-path findings will be closed. Wave 1 complete.

Test plan

  • pytest tests/test_agent_runner.py tests/test_agent_plan.py -v — 15 new pass, 49 pre-existing still green
  • Full Wave 1 regression — 148 tests pass
  • ruff check — no new errors
  • Manifest current
  • TDD: 10 of 15 new tests watched fail RED before implementation
  • CI green on PR
  • Manual verification:
    • Inject Action with path ~/Documents/../../etc/passwd and grant ~/Documents/** → PermissionViolation
    • Check ~/.codec/audit.log for permission_gate_blocked entry with reason=path_traversal
  • Mickael chat review

🤖 Generated with Claude Code

…5+D-14+D-16)

Closes three Phase 1 Audit D findings in one cohesive change to the
Step 9 agent runtime:

  D-5  CRITICAL  permission_gate accepts path-traversal via fnmatch
  D-14 MEDIUM   _PATH_BLOCKLIST_SUBSTRINGS misses ~/.codec/skills + plugins
  D-16 MEDIUM   blocklist substring match is anchorless

All three are auto-extract / runtime paths that an LLM-drafted plan
could use to chain into D-1 RCE. Each layer closes independently.

codec_agent_runner.py — permission_gate refactor (D-5):
- New helper _path_allowed(action_path, grants) → (bool, reason):
  * reject `..` segments outright (closes the dotdot bypass that
    fnmatch glob-matched against grants like ~/Documents/**)
  * os.path.realpath both sides — symlink-out-of-grant rejected
  * fnmatch replaced with action_real.startswith(grant_real + os.sep)
    Trade-off: a grant like ~/Documents/*.md now accepts any file
    under realpath(~/Documents/) — safety > granularity per audit.
- New helper _emit_gate_blocked(action_path, reason, agent_id):
  emits `permission_gate_blocked` audit event before raising
  PermissionViolation. source=codec-agent-runner, outcome=error,
  level=warning, extra={requested_path, resolved_path, reason,
  agent_id}. Forensic visibility per D-5 closure §3.
- Removed unused `import fnmatch` (no other callers in the module).

codec_agent_plan.py — blocklist extension + segment-aware (D-14+D-16):
- _PATH_BLOCKLIST_SUBSTRINGS extended with 8 new entries (D-14):
    /.codec/skills          /.codec/plugins
    /.codec/oauth_state.json /.codec/audit.log
    /.codec/agents          /.codec/agent_global_grants.json
    /.codec/config.json     /.codec/memory.db
- New helper _path_segments_match(path, pattern) does segment-aware
  matching (D-16): splits both `path` (after expanduser+normpath) and
  `pattern` on `/`, requires consecutive subsequence of segments.
    `~/Documents/notes_ssh/foo.md` no longer matches `/.ssh` (segment
    is `notes_ssh`, not `.ssh`), but `~/.ssh/config` still does.
- New helper _is_path_blocklisted(path) walks the full blocklist.
- extract_user_paths now calls _is_path_blocklisted instead of the
  raw substring `any(b in raw for b in _PATH_BLOCKLIST_SUBSTRINGS)`.

Tests:
- tests/test_agent_runner.py +5 new (143 LOC):
    test_permission_gate_rejects_path_traversal_dotdot
    test_permission_gate_rejects_read_path_traversal
    test_permission_gate_rejects_symlink_outside_grant
    test_permission_gate_accepts_realpath_within_grant
    test_permission_gate_emits_blocked_audit_event
- tests/test_agent_plan.py +10 new (80 LOC, 7 parametrized + 3 misc):
    test_extract_user_paths_blocks_sensitive_codec_dirs (×7)
    test_extract_user_paths_segment_aware_not_false_positive
    test_extract_user_paths_still_blocks_real_ssh_path
    test_extract_user_paths_allows_legitimate_user_paths

Verification:
- pytest tests/test_agent_runner.py tests/test_agent_plan.py
  tests/test_file_write.py tests/test_skill_routes.py
  tests/test_skill_registry.py tests/test_skill_contracts.py
  → 148 passed (full Wave 1 regression + new)
- python3 tests/test_skill_imports.py → 76 parsed, 0 errors
- python3 tools/generate_skill_manifest.py --check → ok
- ruff check codec_agent_runner.py codec_agent_plan.py
  → 5 errors in codec_agent_runner.py + 22 in codec_agent_plan.py,
    all pre-existing on main (F401 unused imports `field`, `time`,
    E402 inline imports for regex, F541 f-string-without-placeholder,
    F821 List/Tuple undefined). The one new error introduced —
    `fnmatch` no longer used in codec_agent_runner.py — has been
    cleaned up (import removed).

Docs:
- AGENTS.md §7 new "Agent permission gate + path blocklist
  (Phase 1 Wave 1, PR-1D — closes D-5 + D-14 + D-16)" subsection
  documenting the four-layer defense (PR-1A + PR-1C + PR-1D blocklist
  + PR-1D runtime gate) against the D-1 RCE chain.
- docs/audits/PHASE-1-SECURITY.md: closure footnotes on D-5, D-14,
  D-16.
- docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md: D-5 row flipped to
  W1 — CLOSED (PR-1D). D-14 and D-16 are MEDIUM (not in the
  CRITICAL findings table) but their D-section footnotes carry the
  closure trail.

Wave 1 status after this PR merges:
- D-1 ✅ closed (PR-1A, #42, 48ec5d5)
- D-2 ✅ closed (PR-1B, #43, ff16664)
- D-3 ✅ closed (PR-1B, #43, ff16664)
- D-4 ✅ closed (PR-1C, #45, 0065d90)
- D-5 ✅ closed (this PR)
- D-14 ✅ closed (this PR — bonus)
- D-16 ✅ closed (this PR — bonus)

All five CRITICAL skill-loading + write-path findings are closed.

Sample audit line emitted on a blocked traversal attempt:

  {"ts": "2026-05-17T...Z",
   "schema": 1,
   "event": "permission_gate_blocked",
   "source": "codec-agent-runner",
   "outcome": "error",
   "level": "warning",
   "message": "permission_gate refused '~/Documents/../../etc/passwd': path_traversal",
   "extra": {
     "requested_path": "~/Documents/../../etc/passwd",
     "resolved_path": "/etc/passwd",
     "reason": "path_traversal",
     "agent_id": ""
   }}

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@AVADSA25 AVADSA25 force-pushed the fix/pr1d-permission-gate-realpath branch from 57cbd37 to 96d5924 Compare May 17, 2026 16:42
@AVADSA25 AVADSA25 merged commit fd2b460 into main May 17, 2026
1 check passed
AVADSA25 added a commit that referenced this pull request May 17, 2026
#48)

PR-1D (#47) merged to main as squash commit fd2b460. Update the
closure footnotes for D-5, D-14, and D-16 in
docs/audits/PHASE-1-SECURITY.md plus the D-5 row in
docs/audits/PHASE-1-CONSOLIDATED-TRIAGE.md, replacing the
branch-name placeholders with PR number + commit hash.

Mirrors the citation style applied to:
  D-1   PR-1A #4248ec5d5
  D-2/3 PR-1B #43ff16664
  D-4   PR-1C #450065d90
  D-5   PR-1D #47fd2b460  (this commit)

After this lands, Wave 1 is fully closed with complete citation
trails. All five CRITICAL skill-loading + write-path findings
(D-1, D-2, D-3, D-4, D-5) plus the two bonus mediums (D-14, D-16)
carry merge commit hashes in their audit-doc footnotes.

Co-authored-by: Mickael Farina <farina.mickael@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants