Skip to content

Scope writes by owner, and give the grant it already promised - #494

Merged
ptr727 merged 4 commits into
developfrom
feature/write-guard-owner-scope
Aug 1, 2026
Merged

Scope writes by owner, and give the grant it already promised#494
ptr727 merged 4 commits into
developfrom
feature/write-guard-owner-scope

Conversation

@ptr727

@ptr727 ptr727 commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Resolves the gap reported in gh-write-guard-override-gap.md: rule 3's deny message promised a per-session permission path that did not exist in the code, so a maintainer's explicit, repeated authorization could not be honored.

Verified before changing anything

Claim Result
Rule 3 denies unconditionally True. The logic is inline in classify, not in a function named _check_cross_repo as the report quotes, so the report's code excerpt is a paraphrase. The message and behavior are quoted correctly.
GOVERNANCE.md is absent from HomeAutomation-Config, PhotoCleaner, PlexCleaner True, all three 404. The deny message cited a file the reader cannot open.
The installed hook might be an older revision False. The installed copy differs from this one by a single British spelling (judgement), so behavior is identical.

Why other repos wrote cross-repo without complaint

The report asks this to be reconciled, and it is not a permissions difference. Rule 3 has a narrow trigger, measured against the installed hook:

Command shape Decision
explicit --repo, sibling repo, same owner DENY
explicit --repo, different owner DENY
no explicit target, so the target is the cwd's origin ALLOW
explicit target, cwd is not a git checkout ALLOW
explicit repos/owner/repo API path DENY

So an agent that cds into the other repo and runs a plain gh issue create performs the same write invisibly, and an agent whose cwd does not resolve to a git origin skips the check entirely by design. HomeAutomation-Config's agent was not stricter, it used the one shape the guard can see. The hook is also matcher: "Bash" only, and was installed here on 2026-07-31, so nothing before that and no non-Bash path was ever covered.

What changes

The boundary moves to where the harm is. The incident was a stray comment on a stranger's repository, not work across the maintainer's own projects. A sibling repository under the same owner as origin is now allowed. A different owner is denied unless named in GH_WRITE_GUARD_ALLOW, as owner/repo or owner/*.

The grant is a channel the agent cannot use on itself. It is read from the environment the session was launched with. A hook runs as its own process, so an inline VAR=x cmd prefix or an export inside a Bash call never reaches it. A self-test case asserts exactly that, by classifying a command that carries the assignment as text and expecting a deny.

Nine self-test cases cover the matrix: same owner, the API-path form, a different owner with and without a grant, the owner wildcard, a repo grant not extending to that owner's siblings, one owner's grant not reaching another (the original incident, still denied), a malformed grant granting nothing, and the inline-prefix case above.

The prose moves with the code. GOVERNANCE.md "Repository Boundaries and Write Safety" and the claude-md-safety.md kit said "write only to the current project's own repository", which the hook would now contradict. A guard that allows what the rules forbid leaves the agent to guess which one binds, which is the confusion this report is about.

Two things reported, not changed

  1. The no-origin skip is a real hole. When origin does not resolve, rule 3 is skipped and an explicit foreign target is allowed. Tightening it would deny ad-hoc work outside a checkout, so it is flagged for your call rather than changed alongside a loosening.
  2. Rule 3 scans the raw command string, so merely mentioning a write denies. This fired live while probing the guard for this PR: a python3 heredoc containing the text gh issue create --repo ptr727/PhotoCleaner was denied, though it wrote nothing. Rule 1 strips quoted spans before scanning and rule 3 does not. The fix is to require gh in argv command position, the pattern _push_targets already uses for git push, and it deserves its own change with its own cases rather than riding along here.

Verification

  • --selftest passes, 9 new cases plus the existing matrix.
  • Decision boundaries measured by classifying strings against the installed hook, not inferred.
  • prose_lint blocking set clean tree-wide, and clean on changed lines. repo_gate clean, 180 script tests pass.
  • The hook keeps LF endings, and the three prose files keep CRLF.

Not installed to this host. ~/.claude/hooks/gh-write-guard.py still carries the old rule, so the change takes effect after you merge and reinstall. Say the word and I will run the install.

🤖 Generated with Claude Code

Rule 3 denied any explicit target other than the checkout's origin, while its
deny text told the reader that "another repository needs explicit per-session
permission". No mechanism existed to express that permission, so the guard
named a remedy it could not accept, and the maintainer's repeated explicit
authorization could not be honored. The two available outcomes were both bad:
run the command by hand, or edit the hook to get past a block that just fired,
which disables the protection for everything afterward.

The boundary now sits where the harm sits. The incident this guard exists for
was a stray comment on a stranger's repository, not work across the
maintainer's own projects, so a sibling repository under the same owner as
origin is allowed and a different owner is denied. A different owner is
allowed only when named in GH_WRITE_GUARD_ALLOW, as owner/repo or owner/* for
a whole owner.

The grant is read from the environment the session was launched with, which
is the one channel the agent cannot set for itself. A hook runs as its own
process, so an inline VAR=x prefix or an export inside a Bash call never
reaches it, and a self-test case asserts exactly that by classifying a command
that carries the assignment as text. Nine cases cover the matrix: same owner,
API-path form, different owner with and without a grant, the wildcard, a repo
grant not extending to that owner's siblings, one owner's grant not reaching
another, and a malformed grant granting nothing.

The deny text drops the promise it could not keep and names the grant, says to
ask rather than self-grant, and cites the user-level CLAUDE.md as well, since
GOVERNANCE.md is absent from the repos where the message is read.

GOVERNANCE.md and the CLAUDE.md kit move with it, because a hook that allows
what the prose forbids leaves the agent to guess which one binds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 1, 2026 03:59

Copilot AI 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.

Pull request overview

Updates the GitHub write-safety guard and governance prose to allow writes across sibling repositories under the same origin owner, while still denying cross-owner writes unless explicitly granted via an environment allowlist.

Changes:

  • Adjust Rule 3 in gh-write-guard.py to scope allowed explicit gh write targets to the origin owner, with cross-owner exceptions via GH_WRITE_GUARD_ALLOW.
  • Add self-test coverage for the new Rule 3 decision matrix, including allowlist and “inline env prefix doesn’t apply” cases.
  • Align the installed safety kit docs (claude-md-safety.md, host setup README) and committed governance (GOVERNANCE.md) to the new owner-scoped rule.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
host-setup/agent-safety/README.md Updates host safety kit documentation to describe owner-scoped targeting and GH_WRITE_GUARD_ALLOW.
host-setup/agent-safety/gh-write-guard.py Implements owner-scoped Rule 3 with GH_WRITE_GUARD_ALLOW support and expands self-tests.
host-setup/agent-safety/claude-md-safety.md Updates the user-level CLAUDE.md safety block to match owner-scoped targeting and allowlist behavior.
GOVERNANCE.md Updates “Repository Boundaries and Write Safety” to reflect the owner-scoped write boundary.

Comment thread host-setup/agent-safety/gh-write-guard.py
The target extraction matched only `-R value` and `--repo value` with
whitespace, and took the first match rather than all of them. Probing the
reported equals-form gap found it was one of four in the same class, each a
silent bypass of the whole repository scope rather than a near-miss:

  --repo=owner/repo    allowed
  -R=owner/repo        allowed
  -Rowner/repo         allowed
  a second --repo after && in a compound   allowed

The first three are spellings gh accepts and the regex did not, so the flag
that names the target was invisible and rule 3 compared nothing. The fourth
read the harmless first invocation and never looked at the write after the
separator, which is the shape a compound command takes when one target is the
current repo.

The pattern now matches the separator rather than assuming a space, including
the attached short form, with a look-behind so -R does not match inside a
longer token. Extraction iterates every occurrence, matching what the API-path
branch beside it already did.

Five cases cover it, four asserting a foreign owner denies through each
spelling, and one asserting a sibling owner still allows through the equals
form so the fix is not a blanket deny.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 1, 2026 04:05

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

host-setup/agent-safety/gh-write-guard.py:93

  • _EXPLICIT_REPO is currently anchored with a negative lookbehind (?<![\w\-]), which still matches -R... inside quoted arguments (e.g. a --title "-Rowner/repo") or other non-word-delimited text. Since rule 3 scans the raw command string (and does not strip quoted spans the way rule 1 does), this can cause false denies when the text merely mentions -Rowner/repo rather than using it as a flag. Anchoring the regex to token boundaries (start-of-string or whitespace) reduces these false positives without losing coverage of --repo=..., -R=..., or -Rvalue.
# Every spelling gh accepts for the target flag: `--repo x`, `--repo=x`, `-R x`, `-R=x`, and the attached
# short form `-Rx`. A form left out is not a near-miss, it is a silent bypass of the whole repository
# scope, so the separator is matched rather than assumed to be a space. The leading look-behind keeps
# `-R` from matching inside a longer token.
_EXPLICIT_REPO = re.compile(r"(?<![\w\-])(?:--repo[=\s]+|-R[=\s]*)(?P<q>['\"]?)(?P<r>[^\s'\"]+)(?P=q)")

…it does

The flag pattern allowed anything that was not a word character before it, so
a value opening a quoted span (--title "-Rowner/repo") read as a flag and
denied a write that targets nothing. It now has to start a shell token,
whitespace before it or the string start, which is where a real flag sits.

Measured against the previous pattern, the anchor changes exactly one shape
and no real flag:

  --title "-Rowner/repo"        was read as a target, now is not
  --title "use -Rowner/repo"    still read as a target
  -Rowner/repo, --repo=x        unchanged

The second line is the point. A mention inside prose is preceded by a space
like a real flag, so the anchor cannot separate them, and the first draft of
this change carried a case asserting otherwise that failed on the run. Telling
a flag from text needs argv-position parsing, the way _push_targets already
does it for git push, so the comment names that rather than implying the class
is closed. The case asserts the shape that holds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727

ptr727 commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Answering the suppressed finding from round 2. Adopted in b3d40be42dbf21bcf96d2c3b853afaacb454c7ba, with the claim narrowed to what it actually achieves.

The anchor is right and I kept it. It now requires the flag to start a shell token, so your example denies nothing:

                              old pattern              new pattern
--title "-Rowner/repo"        ['esphome/esphome', ...]  ['ptr727/X']     <- fixed
--title "use -Rowner/repo"    ['esphome/esphome', ...]  unchanged        <- NOT fixed
-Rowner/repo                  unchanged                unchanged
--repo=owner/repo             unchanged                unchanged

But it does not close the class, and my first draft claimed it did. I wrote a case asserting that a mention inside a title stops being read as a target, and it failed on the run: a mention mid-prose is preceded by a space, exactly like a real flag, so no look-behind can separate them. The case now asserts the shape that holds, a value opening a quoted span, and the comment names the residual instead of implying it is gone.

Separating a flag from text needs argv-position parsing, the way _push_targets already does it for git push. That is the same open item this PR's description records under "reported, not changed", found when the guard denied a read-only python3 probe whose heredoc merely contained the text. It deserves its own change with its own cases rather than a look-behind that half-covers it.

No coverage is lost: all four bypass spellings from round 1 still deny, and a sibling owner still allows.

Copilot AI review requested due to automatic review settings August 1, 2026 04:11

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

host-setup/agent-safety/gh-write-guard.py:96

  • The comment above _EXPLICIT_REPO says the regex “does not reach a mention inside prose (--title "use -Rowner/repo")”, but the pattern only checks for preceding whitespace (or start of string). That means -Rowner/repo inside a quoted argument can still match when preceded by a space, so the comment is misleading about the current behavior.
# scope, so the separator is matched rather than assumed to be a space. The look-behind requires the flag
# to start a shell token (whitespace before it, or the string start), which is where a real flag always
# sits, so a value that opens a quoted span (`--title "-Rowner/repo"`) is not read as a target. It does
# not reach a mention inside prose (`--title "use -Rowner/repo"`), which is preceded by a space like a
# real flag: rule 3 reads the raw string, so telling a flag from text needs argv-position parsing, the
# way _push_targets does it for git push.

The comment read "It does not reach a mention inside prose", meaning the
anchor does not fix that case. A reviewer read it as the pattern not matching
there, which is the opposite behavior, and in a comment on a safety control an
ambiguity that inverts is a defect whatever the author intended.

It now states the behavior positively: a mention inside prose IS still read as
a target and still denies, because a space precedes it exactly as one precedes
a real flag, so no look-behind separates the two.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ptr727

ptr727 commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Answering the suppressed finding from round 3. Fixed in 1708cd071276eb729a0c48cf60ef0ca1e720304b.

You read the comment as saying the pattern does not match there. I meant the anchor does not reach that case, so the behavior is unchanged and it still denies. Those are opposite readings of one sentence, and in a comment on a safety control an ambiguity that inverts the meaning is a defect regardless of which reading the author intended, so the wording is the thing to fix rather than the record.

It now states the behavior positively rather than as a negation:

A mention inside prose (`--title "use -Rowner/repo"`) IS still read as a target and still denies.
A space precedes it exactly as one precedes a real flag, so no look-behind can separate the two.

The measured behavior is unchanged and matches that sentence, and it is the same residual this PR's description and the previous round both record as needing argv-position parsing.

Copilot AI review requested due to automatic review settings August 1, 2026 04:14

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 8bdfa22 into develop Aug 1, 2026
7 checks passed
@ptr727
ptr727 deleted the feature/write-guard-owner-scope branch August 1, 2026 04:51
ptr727 added a commit that referenced this pull request Aug 1, 2026
…to main (#498)

Promotes eight commits from `develop`. Every one traces to a defect
found in use rather than to a planned change, and five came from
downstream agents reporting what the procedure did not cover.

## What lands

| Commit | Change |
| --- | --- |
| `a0f6f24` (#487) | `docs/content-import.md`, the three capture-pass
findings from the Blog migration: an export is not a media capture, a
sitemap is not the URL contract, and an HTTP fetch is not the original |
| `8bdfa22` (#494) | The write-guard scopes by **owner** rather than
exact origin, with `GH_WRITE_GUARD_ALLOW` for a different owner. Four
target-flag spellings that silently bypassed rule 3 are closed |
| `981d92f` (#492) | The purpose is restated as agent enablement, in
`README.md`, `HISTORY.md` and `AGENTS.md` |
| `275705b` (#493) | `TODO.md` becomes the fleet backlog destination,
and `spec/readme-structure.md` stops mandating the construction
`GOVERNANCE.md` bans |
| `b6ab6db` (#491) | `STANDUP.md` step 0A names the maintainer-only
prerequisites, and step 4 asserts the remote |
| `89e3868` (#488) | The `version.json` floor is chosen at standup
rather than inherited |
| `eed2e00` (#495) | `README.md` gains "What It Achieves", the
objectives and the fidelity ladder |
| `f6dc5e6` (#497) | `CODESTYLE.md` stops claiming CI runs the same
checks as the clean-compile |

## Provenance

Five of the eight answer issues filed by downstream agents: #456 (Blog
intake), #489 and #490 (the Blog carry), and #496 (Financial-Modeling).
That loop is the one "What It Achieves" now describes, and this
promotion is it closing.

## Divergence, checked rather than assumed

`main...develop` reports `8 ahead, 102 behind, diverged`. **The 102 are
topology, not content.** They are 79 promotion merge commits, which are
main-only by construction because a promotion is a two-parent merge,
plus 23 `Update codegen files` bot commits from this repo's pre-2.0 life
as a .NET sample template. `git diff --name-status origin/develop
origin/main` returns exactly the 12 files these 8 commits touch, so
there is **no main-only content** to reconcile.

## Verification on the merged `develop`

Every gate re-run after the last merge, not inherited from the
individual PRs:

- `prose_lint.py . --check charset --check dupword --check spelling`
clean tree-wide.
- `repo_gate.py` clean (`eol`, `sha-pin`).
- `spec/validate.py` clean, 21 cataloged repos classify.
- `gh-write-guard.py --selftest` passes, including the nine new
repository-scope cases.
- `python3 -m unittest discover -s scripts`, 180 tests pass.
- The `README.md` objectives list verified as one continuous list of 8
bullets with the fidelity table nested, since an unindented table split
it in review.

## Merge mechanics

This is a promotion, so it merges as a **merge commit** with two
parents, never a squash. Its head **is** `develop`, so `--delete-branch`
must not be used.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.

2 participants