Skip to content

fix(approval): gate unattended VCS mutations - #60077

Open
izumi0uu wants to merge 1 commit into
NousResearch:mainfrom
izumi0uu:codex/60056-vcs-mutation-approval
Open

fix(approval): gate unattended VCS mutations#60077
izumi0uu wants to merge 1 commit into
NousResearch:mainfrom
izumi0uu:codex/60056-vcs-mutation-approval

Conversation

@izumi0uu

@izumi0uu izumi0uu commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes an approval-boundary gap where unattended Hermes sessions could perform irreversible remote repository mutations without human consent.

This PR adds approval detection for remote VCS/GitHub mutations, propagates an explicit no-human session marker for kanban notification wakeups, and removes repo-shipped guidance that suggested using execute_code as a workaround when the security scanner blocks piped terminal commands.

Related Issue

Fixes #60056

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tools/approval.py
    • Detects gh pr merge, mutating gh api -X/--method POST|PUT|PATCH|DELETE, gh release create/delete/edit/upload, normal git push, and remote ref deletion via git push.
    • Preserves non-mutating exceptions such as readonly gh api and git push --dry-run.
    • Treats explicitly unattended contexts as no-approver jobs and fails closed by default via the existing approvals.cron_mode trust switch.
  • gateway/session_context.py, gateway/run.py, gateway/kanban_watchers.py
    • Adds a context-local HERMES_UNATTENDED_SESSION marker.
    • Marks kanban completion notification wakeups as unattended so they do not queue approval prompts that no human can answer.
  • skills/research/research-paper-writing/references/experiment-patterns.md
    • Replaces the execute_code workaround advice with guidance to rewrite commands, split fetch/parsing steps, or ask for explicit approval.
  • Tests added/updated for VCS mutation detection, unattended command blocking, execute_code blocking, and session-context propagation.

How to Test

  1. Reproduce the old classification on upstream/main: detect_dangerous_command("gh pr merge 60 --squash --delete-branch") returned false, as did mutating gh api, gh release create, normal git push, and git push origin --delete feature.
  2. Apply this PR and verify the same commands are detected while readonly gh api and git push --dry-run stay allowed.
  3. Verify unattended execution blocks instead of silently approving:
    • HERMES_UNATTENDED_SESSION=1 + check_all_command_guards("gh pr merge ...", "local") returns approved=False.
    • HERMES_UNATTENDED_SESSION=1 + check_execute_code_guard(...) returns approved=False.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: macOS / local Hermes worktree

Documentation & Housekeeping

  • I've updated relevant documentation — skill guidance that recommended execute_code as a scanner workaround
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A, no new config key
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact — regex/session-context changes are platform-neutral
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A, approval behavior only

Tests

./.hermes/with-env.sh python -m pytest tests/tools/test_approval.py tests/tools/test_gnu_long_option_abbreviation_bypass.py tests/tools/test_execute_code_approval_cluster.py tests/gateway/test_session_context_inheritance.py tests/tools/test_local_env_session_leak.py -q
# 365 passed

./.hermes/with-env.sh python -m pytest tests/tools/test_approval_deny_rules.py tests/tools/test_cron_approval_mode.py -q
# 46 passed

./.hermes/with-env.sh ruff check tools/approval.py gateway/session_context.py gateway/run.py gateway/kanban_watchers.py tests/tools/test_approval.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_gnu_long_option_abbreviation_bypass.py tests/gateway/test_session_context_inheritance.py
# All checks passed

./.hermes/with-env.sh python -m py_compile tools/approval.py gateway/session_context.py gateway/run.py gateway/kanban_watchers.py tests/tools/test_approval.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_gnu_long_option_abbreviation_bypass.py tests/gateway/test_session_context_inheritance.py

git diff --check

@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets comp/gateway Gateway runner, session dispatch, delivery comp/cron Cron scheduler and job management area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data P2 Medium — degraded but workaround exists labels Jul 7, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: fixes #60056 (autonomous session merged a prod PR with no approver). Part of the unattended-approval-context cluster: #58673 (gateway-hosted cron/kanban skip approvals.cron_mode: deny), #59179 (deny unattended gateway jobs via cron mode), #58662/#59236 (cron marker leaking into interactive sessions). This PR adds git push / gh pr merge / gh api -X POST|PUT|PATCH|DELETE / gh release ... to the dangerous-command patterns and fails closed for unattended contexts. Note: the approval gate is an in-process heuristic (SECURITY.md 3.2), not an OS-isolation boundary crossing, so priced P2 not P0/P1. Reviewer: sibling PR #59179 overlaps the cron-mode routing change here \u2014 pick one.

@embwl0x

embwl0x commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

I audited my later draft #60799 against this PR and am closing mine as a duplicate because this one was submitted first and has the broader unattended-session coverage.

Two concrete security cases from that audit may be worth adding here before merge:

  • gh api graphql -f query='mutation { ... }' mutates remotely without an explicit -X / --method, so the method-only gh api pattern does not catch it.
  • A session-level approval for generic execute_code can cover a script that embeds a remote mutation (gh pr merge, git push, etc.); the remote-mutation key should still require explicit consent rather than inheriting generic code-execution approval.

My draft also covered mutating gh repo subcommands. The relevant regression ideas are in #60799 if useful.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing a real approval-boundary gap. Current main still only detects force-push variants in tools/approval.py:762-763, and its test suite explicitly treats normal git push as safe at tests/tools/test_approval.py:1724-1728.

Problems

  • gateway/run.py:10612 passes event= into _set_session_env, but existing one-argument test doubles were not updated. The PR CI fails gateway suites with TypeError: ... unexpected keyword argument 'event'.
  • tools/approval.py:770 only catches gh api with explicit -X/--method; it misses the discussed GraphQL mutation form without either flag.
  • The generic execute_code session approval remains reusable at tools/approval.py:2817-2821, so it cannot serve as explicit consent for a later raw-Python remote mutation.

Suggested changes

  • Update affected gateway mocks and rerun the failing gateway coverage.
  • Add GraphQL-mutation and generic-execute-code-session regression cases, and ensure remote mutations require their own consent key.

Automated hermes-sweeper review.

Comment thread gateway/run.py
Comment thread tools/approval.py
@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026
Remote repository mutations need a consent boundary even when they are not shell-destructive. This change teaches approval detection about VCS/gh mutation commands and marks kanban notification wakeups as unattended so no-human turns fail closed instead of queuing prompts no one can answer.

Constraint: Preserve existing local headless auto-approve behavior unless the turn is explicitly unattended.

Rejected: Block every headless local command | too broad for existing local automation contracts.

Rejected: Add a new user-facing approval config key | the existing approvals.cron_mode trust switch already models no-approver jobs.

Confidence: high

Scope-risk: moderate

Directive: Keep remote mutation approval semantics path-wide; execute_code and terminal dispatch must not diverge for unattended sessions.

Tested: ./.hermes/with-env.sh python -m pytest tests/tools/test_approval.py tests/tools/test_gnu_long_option_abbreviation_bypass.py tests/tools/test_execute_code_approval_cluster.py tests/gateway/test_session_context_inheritance.py tests/tools/test_local_env_session_leak.py -q

Tested: ./.hermes/with-env.sh python -m pytest tests/tools/test_approval_deny_rules.py tests/tools/test_cron_approval_mode.py -q

Tested: ./.hermes/with-env.sh ruff check tools/approval.py gateway/session_context.py gateway/run.py gateway/kanban_watchers.py tests/tools/test_approval.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_gnu_long_option_abbreviation_bypass.py tests/gateway/test_session_context_inheritance.py

Tested: ./.hermes/with-env.sh python -m py_compile tools/approval.py gateway/session_context.py gateway/run.py gateway/kanban_watchers.py tests/tools/test_approval.py tests/tools/test_execute_code_approval_cluster.py tests/tools/test_gnu_long_option_abbreviation_bypass.py tests/gateway/test_session_context_inheritance.py

Not-tested: Full repository pytest suite.
@izumi0uu
izumi0uu force-pushed the codex/60056-vcs-mutation-approval branch from 97436e4 to c60b2ba Compare July 18, 2026 17:16
@izumi0uu

Copy link
Copy Markdown
Contributor Author

Addressed in c60b2ba. I updated the gateway _set_session_env test doubles for the new event= keyword and added synthetic-event coverage, added detection for gh api graphql mutations without an explicit HTTP method, and gave raw-Python remote repository mutations their own execute_code:remote_repository_mutation consent key so generic session approval cannot authorize them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: autonomous agent merged prod PR with no consent — ungated VCS ops + execute_code approval gap

4 participants