fix(dashboard): guard git review credential mutations

[necoweb3]

Summary

The dashboard Git/review API could mutate credential files through the coding rail/review pane.

Affected routes:

• POST /api/git/review/stage
• POST /api/git/review/revert
• POST /api/git/review/commit

Current behavior on main:

• review_stage(".env") runs git add -- .env
• review_revert(".env") runs git clean -fd -- .env, deleting an untracked credential file
• review_commit() auto-stages everything with git add -A when nothing is staged, which can commit .env into git history

Why

Dashboard file browser/read/write hardening now treats credential paths as protected, but the Git review API is a separate surface. It can stage, commit, or delete those same files without passing through the file safety guards.

This creates credential leakage and credential-destruction risk from an authenticated dashboard/desktop review flow.

Changes

• Add protected-path checks in hermes_cli/web_git.py using:
  • agent.file_safety.get_read_block_error()
  • agent.file_safety.is_write_denied()
• Reject targeted stage/revert operations on protected paths.
• Replace bulk git add -A with staging only non-protected changed paths.
• Replace bulk revert/clean with reverting only non-protected changed paths.
• Map protected Git mutations to HTTP 403 in the dashboard route wrapper.
• Add regression tests for:
  • staging untracked .env
  • reverting untracked .env
  • commit auto-stage skipping .env

Tests

python -m pytest tests/hermes_cli/test_web_server_git.py -k "review_stage_rejects or review_revert_rejects or auto_stage_env" -q --timeout-method=thread
3 passed, 11 deselected

python -m pytest tests/hermes_cli/test_web_server_git.py -k "stage_commit_roundtrip or commit_with_nothing_staged or review_stage_rejects or review_revert_rejects or auto_stage_env" -q --timeout-method=thread
5 passed, 9 deselected

[egilewski]

suggesting changes

The new filter still lets the review commit endpoint commit credential contents when a protected path is renamed or copied to a safe-looking path. _walk_entries() intentionally skips the porcelain-v2 rename origin record and yields only the destination path, then _safe_changed_paths() checks only that yielded destination before review_commit() auto-stages it. I reproduced this on the PR head by creating a repo with a tracked .env, running git mv .env safe.txt, and then calling hermes_cli.web_git.review_commit(...): it returned {"ok": true} and the new commit contained safe.txt with OPENAI_API_KEY=sk-secret. That keeps a credential-leak path open through POST /api/git/review/commit even after this patch.

Security evidence: authenticated dashboard Git/review API calls can issue git mutations inside a user-selected repository; protected credential path contents must not be staged or committed by the review API even when Git reports a rename/copy with a non-protected destination; current main is vulnerable to bulk git add -A; the PR head still has the residual rename/copy-origin bypass; the PR's focused tests passed locally as 5 passed, 9 deselected, including direct .env stage/revert and auto-stage cases; and the residual bypass search found that _walk_entries() drops the old protected path for tag 2, so the protected-path filter cannot see it.

Signed: GPT-5.5-xhigh in Codex

[alt-glitch]

Priority corrected: P1 → P3 (triage calibration).

Assessed against the repo SECURITY.md: this is §3.2 out-of-scope hardening, not a
§3.1 boundary crossing. Guards git-review credential mutations — the operator stages/reverts their own .env; the shell tool already reaches those files. NOTE: the guard is bypassable via rename (egilewski AI review), so the fix is also incomplete, not merely mispriced.

The dashboard is loopback-by-default and its caller is the operator/agent already
inside the trust envelope, so an in-process guard over operator-owned files does not
cross an OS-isolation boundary or expose an unauthorized external surface. Per the
"PoC or GTFO" policy, type/security alone does not earn P0/P1.

Keeping type/security (this is valid low-priority hardening) and adding needs-repro.
To re-escalate: attach a reproducible exploit reachable by an unauthorized external
party and state the in-scope class, i.e. security-scope: in-scope §3.1 <class> with the
attacker path. Without that, priority stays P3.

[necoweb3]

Updated the branch to handle rename/copy origins on the mutation path too.

review_commit now checks staged protected rename/copy origins before committing, and auto-stage only stages changed entries whose full path set is safe. A tracked .env renamed to safe.txt is now rejected instead of being committed under the safe-looking destination.

Validation:
python -m pytest tests/hermes_cli/test_web_server_git.py -q -k "commit" --timeout-method=thread
5 passed, 10 deselected

[egilewski]

suggesting changes

The update fixes the staged git mv .env safe.txt case, but review_commit() can still commit credential contents when the file is renamed outside Git first. I reproduced this on the PR head with a tracked .env, mv .env safe.txt, and then hermes_cli.web_git.review_commit(..., push=False): Git reported a protected .env deletion plus untracked safe.txt, _safe_changed_paths() filtered out .env but still auto-staged safe.txt, and git show HEAD:safe.txt printed the synthetic secret. That keeps POST /api/git/review/commit able to commit protected credential contents under a safe-looking destination.

Security evidence:

• trust boundary: authenticated dashboard review API calls can mutate Git state in a selected repository, but protected credential files must not be committed through review helpers.
• source/sink/invariant: review_commit() auto-stages _safe_changed_paths() when no staged entries exist, so protected content moved to a safe-looking untracked path must be rejected or left uncommitted.
• current-main reproduction: the same filesystem-rename probe on current GitHub main returned {"ok": true} and committed HEAD:safe.txt with OPENAI_API_KEY=sk-secret.
• PR-head or patch-replay validation: the PR head now rejects git mv .env safe.txt, but the filesystem-rename probe still returned {"ok": true} and committed the synthetic secret.
• positive/negative cases: focused PR tests passed as 5 passed, 10 deselected; the positive git mv probe raised PermissionError; the negative filesystem-rename probe still committed.
• residual bypass search: porcelain-v2 reports the filesystem rename as .env deletion plus untracked safe.txt, so the tag 2 rename-origin handling is never reached.
• reviewer validation: local source/probe review found this blocker; reviewer tooling was skipped because local review already found a decisive issue.

Signed: GPT-5.5-xhigh in Codex

[teknium1]

Thanks for extending the guard to porcelain-v2 rename origins. The current-main premise is real: review_commit() still invokes git add -A when nothing is staged (hermes_cli/web_git.py:364-369).

Problems

• Blocking: the PR still auto-stages a safe-looking destination after a plain filesystem move. _walk_entries_with_paths() records an origin only for tag 2 (hermes_cli/web_git.py:223-228), but review_commit() stages every _safe_changed_paths() result (hermes_cli/web_git.py:440-443). A protected tag-1 deletion plus untracked safe.txt therefore leaves safe.txt eligible.
• The same helper drives bulk stage and revert (hermes_cli/web_git.py:403, 417), so the gap is not commit-only.
• The added regression test covers git mv only (tests/hermes_cli/test_web_server_git.py:163-182), not the deletion-plus-untracked status shape.

Suggested changes

• Make the batch policy fail closed for protected-deletion/untracked-destination cases across stage, revert, and auto-stage commit.
• Add a real-repo test using a filesystem move and assert safe.txt is not staged, committed, or deleted.

Automated hermes-sweeper review.

[teknium1]

_safe_changed_paths() cannot recognize a plain filesystem move: porcelain reports the old protected path as a tag-1 deletion and the safe-looking destination as ?, so this stages the destination. Please handle that batch shape and add a regression test before using this list for auto-stage.