fix(files): leading slashes no longer skip the deny list and fences; DELETE carries the deny list (abilityai/trinity-enterprise#792)

[webmixgamer]

Summary

• The backend file routes normalised a user path with posixpath.normpath, which keeps exactly two leading slashes. The agent server's Path.resolve() reads them as one. As a result, a //-prefixed path matched none of the path-anchored deny patterns and not the skills.manage fence. _normalize_user_path now collapses any run of leading slashes to one.
• PUT, POST /mkdir and DELETE /files now send the agent the normalised path they checked, not the raw input. The agent echoes what it receives, so the response path / deleted fields and the agent's 404/409 messages now carry the absolute normalised path. The Files tab does not read them.
• The agent-side copy of the same policy had the same shape: the PreToolUse file-guardrail.py and read-only-guard.py hooks normalised file_path with os.path.normpath, so a // path skipped their absolute patterns. Both now collapse leading slashes. This was found in PR review.
• DELETE /files now applies the backend deny list, which it never did before. It also refuses a directory that holds a path-anchored protected path (.ssh, .claude, the home dir), because deleting the directory removes what is inside. The refusal is 403 Cannot delete protected path: {path}.

Changes

• src/backend/services/agent_service/files.py:
  • the one-line normaliser fix;
  • _deny_anchor / _DENY_ANCHORS, derived from _FILE_WRITE_DENY_PATTERNS so the list is not duplicated;
  • _is_user_deletable_path, wired into delete_agent_file_logic after the access check and the skills.manage fence, and before the container lookup;
  • the normalised path forwarded on all three write routes.
• tests/unit/test_files_protected_paths.py now imports the shipped module; it used to test an inline copy of the functions. It adds:
  • // rows and a delete table;
  • the pinned anchor set, plus the anchor-derivation rule tested on hypothetical glob shapes;
  • three Hypothesis properties: the normalised form equals an independent segment-stack resolution, extra leading slashes never change a verdict, and DELETE is refused exactly when writing is or the path holds an anchor;
  • route-logic checks;
  • a check through the real FastAPI router on all three routes.
• docker/base-image/hooks/file-guardrail.py, read-only-guard.py: _normalise collapses leading slashes. No file is added, so the image's COPY set is unchanged.
• tests/unit/test_ent792_file_guardrail_hook.py (new): runs the shipped hook's main() against the shipped baseline. tests/unit/test_read_only_guard.py: // rows.
• tests/unit/test_ent596_skill_manager.py: // rows on the skills fence, for classification and for the three routes.
• tests/test_files_guardrail_bypass.py (live tier): // traversal rows, DELETE refusals against a stopped agent, and a // mkdir row.
• Docs:
  • feature-flows/file-browser.md and skill-manager-permission.md;
  • requirements/content-files.md 13.1;
  • a learnings fragment;
  • the /cso --diff report.

No migration and no frontend change. The only docker/ change is the two hook functions above.

Behaviour change (Files tab)

These deletes are now refused for every caller, owners included:

• .ssh, .aws, .gcp and everything inside them;
• .claude/settings*.json, and the whole .claude folder;
• .env.* at any depth;
• .credentials.enc;
• anything under /proc.

Folders inside .claude (skills/, agents/, projects/) can still be deleted. The UI shows the refusal as the existing Failed to delete: … toast. The Delete button stays enabled; the frontend is unchanged, and #3277 tracks disabling it.

A directory that merely contains a basename-protected file, such as .env, is not refused. Lexical matching cannot see inside a directory, and the agent server's by-name block still applies to the file itself.

Test Plan

• cd tests && pytest unit/test_files_protected_paths.py unit/test_ent596_skill_manager.py unit/test_ent792_file_guardrail_hook.py unit/test_read_only_guard.py -q: all pass. Together with every unit test file that touches the file routes or the guardrail hooks: 677 passed, random order.
• Mutations, each restored byte-identical. Each one turns tests red:
  • reverting the normaliser line: 30 red;
  • dropping the DELETE check: 11 red;
  • dropping the holding-directory clause: 22 red;
  • forwarding the raw path: 4 red;
  • cutting anchors at the first glob: 2 red;
  • treating a string prefix of an anchor as a parent folder (a.startswith(head)): 5 red;
  • reverting the file-guardrail.py fix: 7 red; reverting the read-only-guard.py fix: 2 red.
• /verify-local --skip-agent --skip-unit (rerun after the hook change, still passing): build and import-smoke, boot and health, test preflight, and integration all pass (70 passed, 13 agent- or PG-gated skips, 2 known false-fails deselected).
  • The full --skip-agent run's unit stage failed: 41 tests in four unrelated files that only fail in the full sequential run (test_ent666_objective_join, test_ent477_git_refresh_hook, test_ent477_definitions_endpoint, test_retention_floor).
  • All four pass alone (120 passed) and none touches the changed module.
• Agent container (trinity-agent-base): //home/developer/…, /proc/self/root/… and /proc/self/cwd/… all resolve into /home/developer.
• Hooks in the real image (worktree hooks mounted read-only into trinity-agent-base, no network):
  • the file guardrail denies //home/developer/.ssh/authorized_keys, //home/developer/.claude/settings.json and ///opt/trinity/hooks/lib.py (exit 2), and allows //home/developer/notes.md;
  • with read-only mode on, //home/developer/.claude/agents/x.md is denied and //home/developer/content/ok.txt is allowed.
  • Before the fix, the shipped hook allowed the // forms (exit 0).
• Manual, on a running agent on localhost (as admin, TOKEN from POST /api/token). Verified on 4dc342003; the head ff6c6ec3a changes only tests and tests/registry.json, so the backend is identical:
  • Files tab with hidden files shown. Deleting .ssh, .ssh/<file>, .claude or .env.example gives the toast Failed to delete: Cannot delete protected path: …, and each is still on disk afterwards. An ordinary file deletes normally.
  • curl -X PUT "http://localhost:8000/api/agents/<agent>/files?path=//home/developer/.ssh/x" -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' -d '{"content":"x"}' returns 403 Cannot edit protected path: //home/developer/.ssh/x.
  • mkdir takes its path in the JSON body: curl -X POST "http://localhost:8000/api/agents/<agent>/files/mkdir" -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' -d '{"path":"//home/developer/.ssh/keys"}' returns 403 Cannot create folder in protected path: //home/developer/.ssh/keys.
  • An ordinary file written through ?path=//home/developer/notes.txt lands on notes.txt, which shows the normalised path is what reaches the agent.

Related

#3236 adds _touches_instructions, which calls the same _normalize_user_path, so it picks up this fix when it rebases. Its test_ent164 classification parametrize can then add the // rows.

Fixes abilityai/trinity-enterprise#792

🤖 Generated with Claude Code

[vybe]

merge-train: batch validated on train #3279 (green).