One copy of the push and merge policy, and the forms that walked past it #372

Merged

branderson merged 4 commits from guards-365 into development 2026-10-06 23:13:50 -07:00

Conversation 1 Commits 4 Files Changed 12

+926 -533

dejima-agent commented 2026-10-06 22:49:39 -07:00

Collaborator

Copy Link

Copy Source

Summary generated by LLM agent

Closes #365.

The push and merge policy was parsed in two places, and the review found bypasses that existed in both plus a divergence between them. The structural answer is the same for all of them, so it comes first.

One copy of the decision

Each rule existed twice: as a PATH shadow over the command (docker/git-guard.sh, docker/forge-guard.sh) and as a PreToolUse hook over the command string a harness reports (hooks/git-push-guard.sh, hooks/merge-guard.sh). The overlap is deliberate and neither can be dropped - a hook sees /usr/bin/git push, which walks past the shadow, and a shadow sees a push from an editor integration or a script, which the harness never reports as a Bash tool call. What they cannot do is disagree, and they had: the shadow skipped -o and its value and read --repo, the hook did neither, so git push -o x origin main was fine to one and a hard refusal from the other for the same invocation in the same session.

docker/guard-lib.sh now holds the decision and all four call it. Each hook's job is to turn a command string into the argv the library reads - which is how the hooks inherit the shadows' indifference to spacing and quoting, and why merge-guard.sh no longer escapes regex metacharacters out of a block pattern: there is no regex left to inject into. entrypoint.sh derives the set of CLIs it links the shadow over from the same function the hook checks a command against, so what is shadowed and what is checked cannot drift apart either.

Where the two readings differed the library takes the stricter one. HEAD resolves to the current branch, which the shadow did not do - git push origin HEAD from a protected branch matched no policy entry and went through.

merge-guard.sh consequently applies more than the merge block now: the repository scoping goes with it. It keeps its name because that is the rule it was created for and the harness manifests declare it, and because the hook checks a command against the shadowed CLI set first - without that it would apply gh's --repo rule to any command carrying that flag, which the shadow cannot do because it only exists for those commands.

The bypasses

--all, --mirror and multi-refspec pushes. Each resolved exactly one branch and checked that, so git push --all origin from a feature branch pushed main, and git push origin feature main checked feature alone. Every positional after the remote is now a refspec and every one is checked; --all is checked against every local branch, since that is what it pushes. --mirror is refused outright - it writes every ref AND deletes the remote refs this checkout does not have, so a mirror from a repository with no main is how the remote's main disappears, and no branch list makes that readable off the command.

--git-dir and --work-tree. Both were consumed and ignored, so a push carrying one resolved the remote in the mounted project, found it allowed, and then ran against a repository whose origin points somewhere else. Refused for a push, and only for a push - they say nothing about any other git command, and a guard that refused git --git-dir X status would be refusing the flag rather than the push. Honouring them would mean reimplementing how they combine with -C and with each other.

gh api. It was scoped only when the endpoint sat under the declared segment, so user/repos, /orgs/<org>/repos and graphql named no repository for the guard to compare and nothing was refused. Under that subcommand the question is now inverted: a target the guard can read has to be present. Telling an endpoint apart from a header value or a field value would mean knowing which of the CLI's flags take a separate value, which is the coupling this guard exists to avoid - asking whether a readable target is there needs no such knowledge.

graphql stays a readable target, because dejima's own forges/github/bin/github-pr reads review threads through it and the API has no other route to them, and a query can only read what the token already could. A mutation does not: it is what an agent reaches for once gh pr merge refuses, and no repository scoping can be read out of a GraphQL document. A field value read from a file (-f query=@doc.graphql) is refused under the same subcommand, since it keeps the document out of the command entirely.

This is a behaviour change an operator may feel. gh api user, gh api rate_limit and any other endpoint outside repos/ are now refused, including the read-only ones. The guard cannot tell which repository they reach, and a fast-fail should be wrong in the loud direction - but if that costs a workflow something real, the inversion is one function (_dejima_forge_path_verdict) and worth revisiting rather than working around.

Tested

bash test/run-confined.sh - the full gate - passes with zero failures, and bash test/shellcheck.sh is clean (the library is covered by the docker/*.sh rule).

The drift test that existed for three push forms now covers nineteen, including every form either parser used to read differently, and it asserts that the agreed set contains refusals as well as allowances - agreement on allowing everything would otherwise satisfy it. A second drift test does the same for the forge pair across ten commands.

The structural guarantee is pinned two ways: the COPY line has its own assertion, because setup() points every guard at the checkout and would hide the line going missing; and neither push caller may read DEJIMA_PUSH_HOSTS or DEJIMA_BLOCKED_BRANCHES in code again, comments excepted, since that prose is what lets an operator read either file on its own. Both halves were mutation-verified.

test/runtime.bats gains one container test, because every git command in a session now goes through a shadow that sources a file the image has to carry: a COPY path that stopped matching would break git outright in every session while the confined suites stayed green. The file-wide pair sets no DEJIMA_PUSH_HOSTS, which fails closed, so the refusal itself is the evidence the policy loaded - and the pass-through half is asserted beside it.

The privileged half was not run locally - this sandbox has no Docker socket by design. Please check that job actually ran rather than queued before treating it as verified.

**Summary generated by LLM agent** Closes #365. The push and merge policy was parsed in two places, and the review found bypasses that existed in both plus a divergence between them. The structural answer is the same for all of them, so it comes first. ## One copy of the decision Each rule existed twice: as a PATH shadow over the command (`docker/git-guard.sh`, `docker/forge-guard.sh`) and as a `PreToolUse` hook over the command string a harness reports (`hooks/git-push-guard.sh`, `hooks/merge-guard.sh`). The overlap is deliberate and neither can be dropped - a hook sees `/usr/bin/git push`, which walks past the shadow, and a shadow sees a push from an editor integration or a script, which the harness never reports as a Bash tool call. What they cannot do is disagree, and they had: the shadow skipped `-o` and its value and read `--repo`, the hook did neither, so `git push -o x origin main` was fine to one and a hard refusal from the other for the same invocation in the same session. `docker/guard-lib.sh` now holds the decision and all four call it. Each hook's job is to turn a command string into the argv the library reads - which is how the hooks inherit the shadows' indifference to spacing and quoting, and why `merge-guard.sh` no longer escapes regex metacharacters out of a block pattern: there is no regex left to inject into. `entrypoint.sh` derives the set of CLIs it links the shadow over from the same function the hook checks a command against, so what is shadowed and what is checked cannot drift apart either. Where the two readings differed the library takes the stricter one. `HEAD` resolves to the current branch, which the shadow did not do - `git push origin HEAD` from a protected branch matched no policy entry and went through. `merge-guard.sh` consequently applies more than the merge block now: the repository scoping goes with it. It keeps its name because that is the rule it was created for and the harness manifests declare it, and because the hook checks a command against the shadowed CLI set first - without that it would apply gh's `--repo` rule to any command carrying that flag, which the shadow cannot do because it only exists for those commands. ## The bypasses **`--all`, `--mirror` and multi-refspec pushes.** Each resolved exactly one branch and checked that, so `git push --all origin` from a feature branch pushed `main`, and `git push origin feature main` checked `feature` alone. Every positional after the remote is now a refspec and every one is checked; `--all` is checked against every local branch, since that is what it pushes. `--mirror` is refused outright - it writes every ref AND deletes the remote refs this checkout does not have, so a mirror from a repository with no `main` is how the remote's `main` disappears, and no branch list makes that readable off the command. **`--git-dir` and `--work-tree`.** Both were consumed and ignored, so a push carrying one resolved the remote in the mounted project, found it allowed, and then ran against a repository whose `origin` points somewhere else. Refused for a push, and only for a push - they say nothing about any other git command, and a guard that refused `git --git-dir X status` would be refusing the flag rather than the push. Honouring them would mean reimplementing how they combine with `-C` and with each other. **`gh api`.** It was scoped only when the endpoint sat under the declared segment, so `user/repos`, `/orgs/<org>/repos` and `graphql` named no repository for the guard to compare and nothing was refused. Under that subcommand the question is now inverted: a target the guard **can** read has to be present. Telling an endpoint apart from a header value or a field value would mean knowing which of the CLI's flags take a separate value, which is the coupling this guard exists to avoid - asking whether a readable target is there needs no such knowledge. `graphql` stays a readable target, because dejima's own `forges/github/bin/github-pr` reads review threads through it and the API has no other route to them, and a query can only read what the token already could. A mutation does not: it is what an agent reaches for once `gh pr merge` refuses, and no repository scoping can be read out of a GraphQL document. A field value read from a file (`-f query=@doc.graphql`) is refused under the same subcommand, since it keeps the document out of the command entirely. **This is a behaviour change an operator may feel.** `gh api user`, `gh api rate_limit` and any other endpoint outside `repos/` are now refused, including the read-only ones. The guard cannot tell which repository they reach, and a fast-fail should be wrong in the loud direction - but if that costs a workflow something real, the inversion is one function (`_dejima_forge_path_verdict`) and worth revisiting rather than working around. ## Tested `bash test/run-confined.sh` - the full gate - passes with zero failures, and `bash test/shellcheck.sh` is clean (the library is covered by the `docker/*.sh` rule). The drift test that existed for three push forms now covers nineteen, including every form either parser used to read differently, and it asserts that the agreed set contains refusals as well as allowances - agreement on allowing everything would otherwise satisfy it. A second drift test does the same for the forge pair across ten commands. The structural guarantee is pinned two ways: the `COPY` line has its own assertion, because `setup()` points every guard at the checkout and would hide the line going missing; and neither push caller may read `DEJIMA_PUSH_HOSTS` or `DEJIMA_BLOCKED_BRANCHES` in code again, comments excepted, since that prose is what lets an operator read either file on its own. Both halves were mutation-verified. `test/runtime.bats` gains one container test, because every git command in a session now goes through a shadow that sources a file the image has to carry: a `COPY` path that stopped matching would break git outright in every session while the confined suites stayed green. The file-wide pair sets no `DEJIMA_PUSH_HOSTS`, which fails closed, so the refusal itself is the evidence the policy loaded - and the pass-through half is asserted beside it. **The privileged half was not run locally** - this sandbox has no Docker socket by design. Please check that job actually ran rather than queued before treating it as verified.

dejima-agent added 4 commits 2026-10-06 22:49:40 -07:00

Put the push and merge policy in one place both layers read ... 2d3a8ef32a

Each rule existed twice: as a PATH shadow over the command, and as a
PreToolUse hook over the command string a harness reports. The overlap is
deliberate - a hook sees `/usr/bin/git push`, which walks past the shadow, and
a shadow sees a push from an editor integration or a script, which the harness
never reports - so neither can be dropped. What they cannot do is disagree,
and they had: the shadow skipped `-o` and its value and read `--repo`, the
hook did neither, so `git push -o x origin main` was fine to one and a hard
refusal from the other for the same invocation in the same session.

docker/guard-lib.sh now holds the decision and all four call it. Each hook's
job is to turn a command string into the argv the library reads, which is also
how the hooks inherit the shadows' indifference to spacing and quoting - and
why merge-guard.sh no longer escapes regex metacharacters out of a block
pattern, since there is no regex left to inject into.

The library takes the stricter reading wherever the two differed: `HEAD`
resolves to the current branch, `--repo` names the remote, and a flag's value
is skipped rather than read as a positional.

entrypoint.sh derives the set of CLIs it links the shadow over from the same
function the hook checks a command against, so what is shadowed and what is
checked cannot drift apart either.

Close the push and api forms that walked past the guards ... 07d863d6dc

Four bypasses, all of them a parser resolving exactly one thing and checking
that (#365).

`--all` and `--mirror` were opaque flags, and only the first positional after
the remote was read. So `git push --all origin` from a feature branch pushed
main, and `git push origin feature main` checked feature alone. Every
positional after the remote is now a refspec and every one is checked; --all
is checked against every local branch, since that is what it pushes. --mirror
is refused outright: it writes every ref AND deletes the remote refs this
checkout does not have, so a mirror from a repository with no main is how the
remote's main disappears, and no branch list makes that readable off the
command.

`--git-dir` and `--work-tree` were consumed and ignored, so a push carrying
one resolved the remote in the mounted project, found it allowed, and then ran
against a repository whose origin points somewhere else. Refused for a push -
and only for a push, since they say nothing about any other git command.
Honouring them would mean reimplementing how they combine with -C and with
each other.

`gh api` was scoped only when the endpoint sat under the declared segment, so
`user/repos`, `/orgs/<org>/repos` and `graphql` named no repository for the
guard to compare and nothing was refused. Under that subcommand the question
is now inverted: a target the guard CAN read has to be present. Telling an
endpoint apart from a header or field value would mean knowing which of the
CLI's flags take a separate value, which is the coupling this guard exists to
avoid - asking whether a readable target is there needs no such knowledge.

graphql stays readable, because dejima's own github-pr reads review threads
through it and the API has no other route to them, and a query can only read
what the token already could. A mutation does not: it is what an agent reaches
for once `gh pr merge` refuses, and no repository scoping can be read out of a
GraphQL document. A field value read from a file is refused under the same
subcommand, since it keeps the document out of the command entirely.

Say that the hooks and the shadows read the same decision ... 4c81605df6

Three documents described the hooks as regex-matching a command string while
the shadows compared argv, which is what made the overlap read as two
implementations rather than two entry points. development.md's table gains the
library; configuration.md says which pushes are refused for being unreadable
rather than checked, since that is the part an operator meets as a refusal.

merge-guard.sh is also no longer only about merges, so configuration.md says
what it applies.

Prove in a container that the shadow can load its policy ... 3474c96f95

Every git command in a session now goes through a shadow that sources a file
the Dockerfile copies in, so a COPY line that stopped matching the path would
break git outright in every session - and the confined suites would stay green,
because they point the guards at the checkout rather than the image.

The file-wide pair sets no DEJIMA_PUSH_HOSTS, which fails closed, so any push
is refused and the refusal itself is the evidence the policy loaded. The
pass-through half is asserted beside it: everything that is not a push has to
reach real git, or the shadow is a broken git rather than a guarded one.

branderson approved these changes 2026-10-06 23:13:45 -07:00

branderson merged commit c7521c7b6d into development 2026-10-06 23:13:50 -07:00

dejima-agent referenced this pull request 2026-10-07 09:36:41 -07:00

Close the two gaps a review found in the guards, and rewrap what was written to a forge's rules #373

dejima-agent referenced this pull request 2026-10-07 17:36:30 -07:00

Three forms that walked past the push and api guards #374

Sign in to join this conversation.

Reviewers

No Reviewers

branderson

Labels

Clear labels

Compat/Breaking

Breaking change that won't be backward compatible

Kind/Bug

Something is not working

Kind/Documentation

Documentation changes

Kind/Enhancement

Improve existing functionality

Kind/Feature

New functionality

Kind/Security

This is security issue

Kind/Testing

Issue or pull request related to testing

Priority

Critical

1

The priority is critical

Priority

High

2

The priority is high

Priority

Low

4

The priority is low

Priority

Medium

3

The priority is medium

Reviewed

Confirmed

1

Issue has been confirmed

Reviewed

Duplicate

2

This issue or pull request already exists

Reviewed

Invalid

3

Invalid issue

Reviewed

Won't Fix

3

This issue won't be fixed

Status

Abandoned

3

Somebody has started to work on this but abandoned work

Status

Blocked

1

Something is blocking this issue or pull request

Status

Need More Info

2

Feedback is required to reproduce issue or to continue work

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

branderson dejima-agent

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dejima/dejima#372