Skip to content

fix(guardrails): keep block-dangerous-git's alias walk inside a 512 KB stack so it reaches its deny #8530

Closed

Description

@kyle-sexton

Refs: #7667, #7923

Agent Brief

Type: Bug
Summary: block-dangerous-git's alias walk reaches its deny on a 512 KB stack instead of overflowing first.

Current behavior:
#7923 changes run-guards.sh so a guard's exit() unwinds before the next guard runs, which keeps a deep guard's deny. That PR could not meet the 512 KB criterion, and its Not verified section says why: "the first guard's alias walk itself overflows below about 700 KB before it exits, so the test uses 1 MB and the default stack." The recursion is the alias re-expansion in plugins/guardrails/hooks/block-dangerous-git.sh (around lines 658-706), capped by HOOK_ALIAS_WORK_MAX=128 (:694). On a stack below about 700 KB the guard crashes (SIGSEGV, exit 139) before it prints its deny. Claude Code blocks only on exit 2, so the crash lets the command through.

Desired behavior:

  • With ulimit -s 512, the 141-hop inline alias chain from fix(guardrails): run-guards loses a deep guard's deny on a small stack (Windows exits 0) #7667 (git -c alias.a1=a2 … -c alias.a141=a142 a1), run through the shipped Bash row, exits 2 with the alias-budget deny.
  • Walks that stay inside the budget keep today's verdicts.
  • The alias walk is iterative, with a hard depth cap, and the guard denies when the cap is hit (GitHub Security & Cost's fix shape).
  • The guard fails closed: any internal error or signal in it exits 2 with a deny message, never another status.

Key interfaces:

Acceptance criteria:

  • The diff adds a test that runs the 141-hop chain through the Bash row under ulimit -s 512 and expects exit 2 and the alias-budget deny.
  • The diff adds a stay-loud test that forces an internal failure or deep recursion in the guard and expects exit 2 with a deny message.
  • plugins/guardrails/hooks/block-dangerous-git.test.sh passes unchanged, apart from added cases.
  • scripts/check-changelog-fragments.sh passes, with one guardrails patch fragment added.
  • CI's test-bash passes on the PR.

Out of scope:


Filed by the Issue Triage session as the follow-up to #7923's unmet criterion. The peer sessions agreed to it under the owner's overnight procedure.

Activity

  1. added
    priority: mediumReal value, no hard deadline; normal backlog flow.
    status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.
    on Oct 20, 2026
  2. added theissue type on Oct 20, 2026
  3. added
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    on Oct 20, 2026
  4. added
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    and removed
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    on Oct 20, 2026
  5. melodic-automation-lanes commented on Oct 20, 2026

    @melodic-automation-lanes
    Contributor

    The work-resolve-issue lane stopped partway through this issue and needs a person.

    Why, in the run's own words:

    The run reported done, but a check the runner ran on the branch failed or could not be read, so it is not finished.

    What the run says it did not do:

    • The runner's check results are missing, unreadable or for another tip of the branch.
    • check-affected could not run: it fails with 'could not create directory of .git/worktrees/tree: Read-only file system', with or without the sandbox
    • The walk is still recursive with a depth cap, not iterative as the issue's desired behavior says. The issue's cap-and-deny shape is met; an iterative rewrite of both guards was not attempted.
    • The guard still uses the 'open' abort posture; I did not switch it to 'closed'. SIGSEGV cannot be trapped in-process, so a crash is prevented by the depth cap rather than caught. Other internal errors still exit 0 with a notice.
    • On a platform without /proc/self/limits the depth cap falls back to the 128 work budget, so the stack protection applies on Linux only
    • The depth cap is 25 on a 512 KB stack, so a chain deeper than that is now denied where it was allowed before; on an 8 MB stack the cap is above the work budget and no verdict changes
    • shellcheck could not be run here; it was not permitted
    • CI's test-bash on the PR

    Partial work is on the branch work-resolve-issue/issue-8530-39382060403-1. A retry continues from it; delete the branch first to start again from the default branch.

    Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/39382060403

    To retry, remove needs-human and add agent-ready again.

  6. added
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    and removed
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    on Oct 20, 2026
  7. added
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    and removed
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    on Oct 20, 2026
  8. melodic-automation-lanes commented on Oct 20, 2026

    @melodic-automation-lanes
    Contributor

    The work-resolve-issue lane stopped partway through this issue and needs a person.

    Why, in the run's own words:

    The run resumed work-resolve-issue/issue-8530-39382060403-1 and reported done with no new commit. The earlier run's work is on that branch, which the runner did not check, so no PR was opened.

    Partial work is on the branch work-resolve-issue/issue-8530-39382060403-1. A retry continues from it; delete the branch first to start again from the default branch.

    Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/39386636889

    To retry, remove needs-human and add agent-ready again.

  9. kyle-sexton commented on Oct 20, 2026

    @kyle-sexton
    MemberAuthor

    Remaining scope after #8563 (merged 2026-10-20T08:07:50Z as 85d44db):

    GitHub Security & Cost agreed to this split.

  10. kyle-sexton commented on Oct 20, 2026

    @kyle-sexton
    MemberAuthor

    The remaining fail-closed requirement moves to #8569. bash cannot trap its own SIGSEGV, so the parent launcher enforces it: exec-bash.mjs --fail-closed maps any guard child outcome other than 0 or 2 to a deny, and only guardrails guard rows pass the flag, because the launcher is shared with formatter and lint hooks. GitHub Security & Cost chose this route. The run-guards unwinding half is #7923, which is held for the owner. This issue can close when #8569 merges.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.priority: mediumReal value, no hard deadline; normal backlog flow.status: confirmedA maintainer confirmed the item is real and in scope; ready to be worked.

    Type

    Fields

    No fields configured for Bug.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions