Harden auto-approve edit gate: treat shell-rc and config-execution files as sensitive - #63438

Following up on this — the two items from the review are addressed:

• Multi-file V4A bypass: 843495c carries each V4A target path through the security decision independently, so a mixed patch containing e.g. ~/.bashrc no longer evaluates as one joined display string. The joined form is retained for display only, as suggested.
• Test coverage: tests/acp/test_edit_approval.py adds 108 lines covering the new protected paths (shell RC / config-execution persistence) and the mixed sensitive + non-sensitive V4A case.

Happy to rebase or adjust anything if the approach isn't what you had in mind.

suggesting changes

The patch improves ACP edit approval for V4A targets and several sensitive locations, but executable shell startup/configuration files outside the current sensitive-name and directory rules can still be auto-approved. A model-controlled edit to one of those files can persist code without an ACP permission request.

• [P2] Executable shell startup/configuration files outside the sensitive policy remain auto-approved
  The approval predicate recognizes a fixed set of names and directory components, but not all supported shell startup/configuration locations. In session mode this leaves unlisted executable startup files outside the approval prompt, so a later shell invocation can execute attacker-controlled content.
  Remediation: Use one canonical execution/configuration-file policy shared with the file tools and cover Fish, Nushell, PowerShell, and other supported shell startup locations (or default to approval for executable home configuration). Add positive tests for every supported shell family and negative tests proving each is denied under both non-ask modes.

Security evidence:

• trust boundary: ACP model-controlled write and patch operations pass through the edit-approval decision before the file-write sink.
• source/sink/invariant: Sensitive targets force a prompt; the invariant remains incomplete for executable startup/configuration files outside the current allowlists.
• current-main reproduction: The current implementation accepts an unlisted nested startup/configuration target under non-ask session policy while denying listed sensitive startup files.
• PR-head or patch-replay validation: The patch preserves individual targets and blocks listed sensitive and outside-workspace targets, but does not add coverage for the unlisted startup/configuration family.
• positive/negative cases: Coverage includes shell/login files, credential/VCS/persistence locations, mixed targets, safe workspace files, and outside-workspace targets; unlisted startup/configuration files remain accepted.
• residual bypass search: Approval builders, patch target extraction, and write dispatch are covered; the remaining gap is the unlisted executable shell/configuration startup family.
• reviewer validation: Source review and regression coverage confirm the residual family is still auto-approved.

Review setup: I reviewed a run-owned local rebase or patch replay against current GitHub main because the submitted branch is stale or conflicted; this does not mean the submitted branch itself merges cleanly.

Not checked:

• Full repository test suite
• Ruff validation
• Windows-specific runtime validation

Signed: GPT-5.6-luna-max in Codex

Thanks for the review, this is a fair catch and I agree with it. A name-based allowlist was the
wrong shape for this and an unlisted startup file is exactly the case it misses.

Before I push a fix I want to check the intended scope with you, because I think the gap is wider
than this PR.

The canonical policy you pointed me at, _SHELL_RC_FILES in tools/approval.py, currently matches
bashrc, zshrc, profile, bash_profile and zprofile. So if I make the ACP path share it as
written, the behaviour you flagged does not change, it just moves to one place. Fish, Nushell and
PowerShell startup files are not covered there either. Searching the repo, config.fish appears only
in scripts/install.sh, nushell only in tools/environments/local.py, and I get no hits at all
for Microsoft.PowerShell_profile or PROFILE.ps1.

If I am reading that correctly it would also affect the shell-command path and not just ACP edits,
though you would know far better than I would whether something else covers that case.

I did not want to touch a shared predicate the command path depends on, so I have not changed it. My
default, unless you would rather otherwise, is to keep this PR narrow: fix the ACP path against a
property-based check rather than a name list, and leave anything to do with the shared policy alone
for you to scope. If it is useful I can write up the command-path observation as a separate issue so
it is on record, or leave it entirely, whichever you prefer.

Happy to go a different direction if you have one in mind. It is your call and I do not want to
widen the diff on you.

Two smaller things:

• On staleness, you are right that the branch is behind, currently by about 7700 commits, and I will
  rebase. For what it is worth acp_adapter/edit_approval.py itself has not changed on main since 12
  July, so the rebase should be mechanical rather than a rewrite.
• You noted Windows runtime validation was not checked. I develop on Windows, so I can cover the
  PowerShell profile locations properly, including the per-host profile paths and the
  $PROFILE.AllUsersAllHosts case. I will include that in whichever option you pick.

Let me know which scope you want and I will get it up.

AI code review — automated review for reference, author can ignore or act on any point.

Harden auto-approve edit gate: treat shell-rc and config-execution files as sensitive

1. SENSITIVE_AUTO_APPROVE_DIR_PARTS covers .git, .ssh, autostart, systemd — but not the credential dirs the sibling dashboard hardening treats as sensitive (.aws, .kube, .gnupg, .docker, .azure in hermes_cli/web_server.py's _SENSITIVE_MANAGED_DIR_NAMES). Under the session policy, an edit to ~/.aws/credentials or ~/.kube/config would still auto-approve. Consider aligning the two sets.

2. The home-dotfile catch-all in _is_sensitive_auto_approve_path resolves symlinks before comparing parents (expanded.resolve(strict=False).parent == Path.home().resolve(strict=False)), so a home dotfile that is a symlink into another directory (e.g. ~/.mylogin → /opt/tool/rc) escapes the catch-all — its resolved parent is not home. The enumerated name set covers the common ones, but the catch-all's resolution order leaves an evasion path; checking the unresolved parent too would close it.

3. V4A parsing: _proposal_for_patch_v4a captures target_paths from the parsed paths, but the new tests only exercise *** Update File: directives. V4A patches also support Delete/Rename directives — if those paths aren't captured, a patch that renames or deletes a sensitive path could under-report target_paths and auto-approve. Worth verifying the parser covers every directive type.