Skip to content

fix(pstackpolicy): close merge bypasses and scope push refspecs to their command - #35

Merged
AlexNegrescu26 merged 4 commits into
mainfrom
fix/pstackpolicy-bypasses
Oct 31, 2026
Merged

AlexNegrescu26 merged 4 commits into
mainfrom
fix/pstackpolicy-bypasses

Conversation

@AlexNegrescu26

@AlexNegrescu26 AlexNegrescu26 commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

pstackpolicy is the only thing that stops a headless pstack subagent from landing work without the operator's PSTACK_LANDING_GRANT. A test audit found five merge or trunk-write commands it let through, one false positive that refused ordinary feature pushes, and tests too loose to notice either. This PR closes the holes, removes the false positive, and rewrites the tests so each blocked command names the rule that must fire. The second policy commit, 3ea983e, closes the four gaps the first round listed as open.

For an agent driving the port, a feature push chained with a later read now runs (git push origin feature && git rev-parse HEAD), inside a subshell or brace group too ((cd x && git push origin feature && git rev-parse HEAD)). These now stop at the grant:

  • gh pr -R o/r merge 12 and gh pr --repo o/r merge 12. The same flag position also reached edit --base, close, review, and every other gh group the policy names (issue, release, secret, variable, workflow, repo). gh 2.102.1 resolves gh <group> -R o/r <verb> to the verb for pr, issue, release, secret and workflow, checked with --help.
  • gh api repos/o/r/merges ... and the GraphQL mergeBranch mutation, plus curl to /merges.
  • gh api -X PATCH repos/o/r/git/refs/heads/main ..., a trunk ref moved without a push.
  • The GraphQL enablePullRequestAutoMerge mutation.
  • gt merge.
  • git push origin main; echo done and (git push origin main; true). Both were read as a push of main; and allowed.
  • The GraphQL updateRef, updateRefs and deleteRef mutations, through gh api or curl.
  • git push origin $(git branch --show-current), and any push destination built from $VAR, ${VAR}, a backtick substitution or a * pattern. Each may be trunk, so each is refused like HEAD.
  • A trunk push on the line after a comment that holds an apostrophe (# don't ...).
  • A push inside backticks (echo `git push origin main`), and the same push in an eval cell's Bun.$`...`.

Changes

extensions/pstackpolicy/index.js

  • One gh(group, verbs) pattern builder lets flags sit before the group and between the group and the verb. Every gh rule uses it.
  • API_MERGE lists every API spelling of a merge. The gh api merge rule and the curl rule share it.
  • New rule "moving a trunk branch through the API". It fires on an explicit non-GET method against git/refs/heads/<trunk>. A read, an explicit GET, and a PATCH to a feature ref stay allowed, matching the push rule's treatment of feature branches.
  • New rule "moving or deleting a ref through the GraphQL API, which may be trunk". updateRef and deleteRef name the ref by node ID and updateRefs names it inside an input list, so no text rule can tell trunk from a feature branch. The rule refuses each of them through gh api or curl, wherever it points. A GraphQL read of a ref stays allowed.
  • The merge-when-ready rule moved ahead of the plain merge rule. Before, the plain rule matched every --auto merge first, so this rule's reason never fired.
  • The git push and local-history scans read each simple command on its own. simpleCommands cuts at &&, ||, ;, |, & and newline, inside a subshell or brace group as well. A ( or { counts as a group only at the start of a command. It does not cut inside quotes, backticks, a substitution or a list literal, or at the & of 2>&1 or &>. Every miss leaves text joined, which fails closed.
  • A comment runs from a word-initial # to its newline, whatever quotes it holds. It is dropped without cutting the command around it, so a Python list literal with a comment inside stays one command. $# and ${#x} are not comments.
  • shellTokens ends a word at the same separators, so a glued ; no longer hides a refspec.
  • A push destination containing $ or * resolves only when it runs, so it is refused as "pushing a refspec that resolves only when it runs, which may be trunk". HEAD gets the same treatment it had. When another refspec in the push is certainly trunk, that reason wins. The bare and colon refspec forms now share one destination check.
  • normalize spells a backtick substitution as $ and its command, which is the text a $(...) already left. A backtick refspec then reads as run-time, and a push inside backticks reaches the push scan.
  • A redirection and its target are not refspecs, so git push origin feature > "$LOG" stays allowed.
  • DENY_REASONS is gone. Nothing read it but a test that could not fail.

Heredoc decision. A heredoc body counts. Each body line is scanned as its own command, because bash <<EOF runs it and a heredoc often writes a script that runs later. The forge-mutation regexes keep scanning the whole text, so gh pr merge inside a heredoc body is still refused. A later line never lends its words to an earlier push.

extensions/pstackpolicy/index.test.js

  • The fake host stores handlers by event name and calls tool_call. The old fake ignored the name and passed every test with the handler registered on tool_result.
  • MUST_BLOCK is [command, reasonFragment] pairs, run with test.each. A command refused by the wrong rule fails.
  • One case per bypass and per false positive above. Fail-closed guards cover the segmentation. A redirection is not a separator, a $(...) substitution stays with its push, a heredoc body that runs a trunk push is refused, and a list literal split over lines in an eval cell is still one command. The second commit adds guards for a trunk push in a subshell, a substitution inside a subshell, $#, a redirection target holding $LOG, a GraphQL ref read, and a Bun.$ feature push.
  • The eval cases run with test.each, so each reports its own failure.
  • The merge endpoint case uses PUT, the method GitHub's merge endpoint takes.

extensions/potetomode/index.test.js

  • /poteto-mode <task> pins the session and sends the trimmed task text with its case kept. When the agent is busy it sends with deliverAs: "followUp". /poteto-mode status sends nothing and leaves the pin as it was. Removing the forwarding line used to pass both the unit tests and smoke.

README.md lists the new rules, the flag position, the push scoping, the run-time refspec rule, comments, groups, redirections, and the heredoc decision.

Gaps still open

A throwaway probe found each of these allowed on 84e53e9 and still allowed on 3ea983e.

  • In an eval cell, a JavaScript // comment that holds an apostrophe hides the next line's push. Only # comments are modelled.
  • In an eval cell, a Python f-string push such as subprocess.run(f"git push origin {b}", shell=True) is missed, because the f prefix glues onto git. An identifier refspec in a list literal, as in ["git", "push", "origin", branch], reads as a literal branch name and is allowed.
  • curl https://api.github.com/repos/o/r/merges -d '...' is allowed. curl POSTs because of -d, and the curl rule keys only on -X, --request and --data.
  • A GraphQL mutation read from a file (gh api graphql -F query=@file) is invisible to every text rule.

These err toward refusing.

  • A destination with an expansion is refused even when its literal prefix rules out trunk, as in git push origin "feature/$TICKET". This is new in 3ea983e.
  • A subshell after a keyword is not read as a group, so if true; then (cd x && git push origin feature && git rev-parse HEAD); fi is still refused.

Verification

All commands ran in this worktree on bun 1.4.4 and gh 2.102.1. The first round ran on omp 18.14.1 and the second on omp 18.14.2.

Second policy commit, 3ea983e.

  • cd plugins/pstack && bun test ./extensions gave 179 pass, 0 fail.
  • The CI form, bun test ./extensions/potetomode/index.test.js ./extensions/pstackpolicy/index.test.js, gave 179 pass, 0 fail.
  • With the new tests in place I ran git stash push -- plugins/pstack/extensions/pstackpolicy/index.js, then bun test ./extensions, then git stash pop, each on its own. Against 84e53e9's index.js the suite gave 161 pass, 18 fail. The 18 are every new case. Four are GraphQL ref mutations, six are run-time refspecs, one is a push inside backticks, two are trunk pushes after an apostrophe comment, three are false positives (the subshell, the brace group, a comment that names main), and two are eval cells (a Python list holding a comment, a Bun.$ trunk push). The 6 guards pass on both sources. After the pop the suite gave 179 pass, 0 fail.
  • A throwaway mutation harness (not committed) ran bun test on a temp copy after each of 13 one-change edits to the new source. All 13 made tests fail. The edits were no backtick rewrite, the GraphQL rule dead, the GraphQL rule without curl, no $ check, no * check, an unreadable refspec returning before a certain trunk one, no redirection filter, no comment handling, # anywhere as a comment, a comment that cuts the command, every bracket opaque, every ( and { a group, and brace groups opaque.
  • bash omp-port/check-port.sh printed check-port: PASS.
  • OMP_SMOKE_REQUIRED=1 bun omp-port/smoke-plugin.mjs printed PASS omp/18.14.2: 60/60 skills registered; /poteto-mode on/off recorded; pin reloaded on new_session; no model turn.

First policy commit, 59b23ec, with the potetomode tests in 84e53e9.

  • cd plugins/pstack && bun test ./extensions gave 151 pass, 0 fail, and so did the CI form.
  • The stash run against the older source gave 110 pass, 31 fail. All 19 new bypass cases fail, the two glued-separator cases among them. All 7 false-positive cases and the multi-statement eval case fail. 4 existing cases fail by reason. The two --auto cases fail because the old code fired the plain merge rule, which is the dead rule. The mergePullRequest and merge-endpoint cases fail only because their reason was renamed from "merging a pull request through the API" to "merging through the API". The 6 fail-closed guards pass against the old source, which never cut a command line. They guard the new cut, and the mutation run below shows the naive cut failing the redirection, substitution and list-literal guards. The two heredoc cases pin the decision above.
  • A throwaway probe over the brief's commands (not committed) printed allow for all 9 must-block commands and BLOCK for the chained feature push before the change. After the change it printed BLOCK for all 9 and allow for the chained push.
  • A throwaway mutation harness (not committed) ran bun test on a temp copy after each of 15 one-change edits. All 15 made tests fail. The edits were a naive splitter, no splitter, no glued-separator split, & as a separator inside a redirection, the auto rule moved back behind the merge rule, the handler registered on tool_result, gt merge dropped, the trunk-ref rule dropped, the trunk-ref rule ignoring the method, flags allowed only before the group, task forwarding removed, always followUp, never followUp, status forwarded as a task, and the task text lowercased.
  • gh <group> -R o/r <verb> --help for pr merge, issue close, release create, secret set and workflow run each printed that verb's help.
  • bash omp-port/check-port.sh printed check-port: PASS.
  • OMP_SMOKE_REQUIRED=1 bun omp-port/smoke-plugin.mjs printed PASS omp/18.14.1: 60/60 skills registered; /poteto-mode on/off recorded; pin reloaded on new_session; no model turn.

…eir command

gh reads flags between a command group and its verb, so `gh pr -R o/r merge`
passed every rule anchored on `gh pr merge`. Every gh rule now goes through one
builder that allows flags on both sides of the group.

The API rules missed `/merges`, the `mergeBranch` and
`enablePullRequestAutoMerge` mutations, and a PATCH that moves a trunk ref.
`gt merge` was missing from the Graphite rule. The merge-when-ready rule sat
behind the plain merge rule, so its reason never fired. It now runs first.

The push and local-history scans read every word after `git push` to the end
of the text, so `git push origin feature && git rev-parse HEAD` was refused as
a push of HEAD. They now read one simple command at a time. The cut skips
quotes, backticks, brackets and redirections so it fails closed. A heredoc
body line counts as a command. A separator glued to a word also ends it, which
closes `git push origin main; echo done`.

Tests look the handler up under `tool_call`, assert the rule each blocked
command fires, and add one case per bypass and false positive. The
DENY_REASONS export and its tautological test are gone.
Removing the forwarding passed both the unit tests and smoke. The fake host now
records sendUserMessage and exposes isIdle. `/poteto-mode <task>` must pin and
send the trimmed text, use `deliverAs: "followUp"` while a turn runs, and
`status` must send nothing and keep the pin.
Closes the four gaps PR #35 left open.

- GraphQL updateRef, updateRefs and deleteRef through gh api or curl are
  refused. They name the ref by node ID or inside an input list, so no
  text rule can tell trunk from a feature branch.
- A push destination with `$`, a backtick substitution or a `*` pattern
  resolves only when it runs and may be trunk. It is refused with a
  "may be trunk" reason, like HEAD. A refspec that is certainly trunk
  still wins the reason. normalize spells a backtick substitution as
  `$` and its command, so a push inside backticks is scanned as a push.
- A comment runs from a word-initial `#` to its newline whatever quotes
  it holds, and is dropped without cutting the command it sits in.
- A subshell or brace group at command start separates its commands
  like unparenthesised text. A substitution or list literal still
  keeps its text together.
- A redirection and its target are not refspecs, so `> "$LOG"` does
  not read as a run-time refspec.
- The eval cases are one test each.
@AlexNegrescu26
AlexNegrescu26 merged commit 6c25490 into main Oct 31, 2026
2 checks passed
@AlexNegrescu26
AlexNegrescu26 deleted the fix/pstackpolicy-bypasses branch October 31, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant