Recognise a protected repository by its origin, and fail loudly on a misplaced checkout #235

Merged

vnprc merged 10 commits from agent/rails-remote-identity into master

2026-09-27 17:43:48 +01:00

allod-agent commented

2026-09-27 16:20:56 +01:00

Member

Owner summary

What changes on a running machine: a protected repository is now recognised by the repository its origin names, not only by where it is checked out. Checked out in the wrong place, allod change begin and allod change record refuse (exit 8) and the git hook blocks commits, pushes, rebases and merges on the protected branch; each message names the expected and the actual path.
Also changes: inside a worktree the hook now applies the branch and signing rules of the repository the worktree belongs to, and repo-local hooks run there. A branch list that is present but cannot be read stops the operation instead of reading as "nothing is protected".
What stays identical: a repository at its listed path, a repository whose origin matches no entry, a repository with no origin, and a machine with no branch list all behave as before. Work on agent/* branches in worktrees is not blocked.
What proves it: one table of 43 cases runs against both the Go tool and the bash hook; the hook suite (89), the allod change suite (270) and the Go tests pass, and the hook suite goes red when the remote match is disabled.
Not proven: the hook as deployed. It reaches a machine through allod/archetypes, so nothing changes anywhere until that input is bumped and home-manager rebuilds. After the rebuild, confirm by hand: a commit on master from a worktree of a protected repo is blocked, and a commit on an agent/* branch there goes through.
If it is wrong: revert this PR, or hold the allod/archetypes bump. The forge still rejects an agent's push to a protected branch whatever the local rails do.

Closes allod/tools#112

What was decided

The rule lives twice, held together by one table. The tool is Go and the hook is a single bash file that allod/archetypes copies alone to ~/.config/git/. Of the three packaging options the issue thread lists, this takes accepted duplication: no change in allod/archetypes, no second deployed file, and no dependency of the hook on the allod binary. tests/fixtures/protection-cases.tsv is read by cmd/allod/protection_test.go and by tests/git-hooks/protected-refs-policy.sh, so the two cannot drift without a red test.

The rule. The lookup key is the repository a checkout belongs to: for a linked worktree the main repository, otherwise the checkout itself (which is also right for a submodule and for --separate-git-dir). An entry whose path equals that key answers first. Failing that, owner/repo is taken from git remote get-url origin, and an entry whose path is owner/repo or ends in /owner/repo means the checkout is misplaced. The remote identity is compared without case, as the forge does; the path match stays exact.

What each rail does with a misplaced checkout. The tool refuses begin and record outright, on any branch, since the fix is to move the checkout. The hook blocks exactly what it would block on the protected branch at the right path, and requires signing likewise, and leaves other branches alone.

A new flake check, git-hook-suites. Nothing ran tests/git-hooks/*.sh automatically, so the bash half of the shared table would never have been exercised.

Risk

R3. The hook sits on the path of every commit and push in every repository on a dev machine, and the agent cannot exercise it as deployed. The worst credible failure after these checks pass is the hook refusing legitimate work on some repository state no fixture covers, which a human undoes by reverting and rebuilding; the other direction, falling open, leaves the forge's server-side protection in place. No state is destroyed either way.

Known limits:

The remote identity is the last two components of the origin URL, as the issue specifies. Two different repositories on a host with nested namespaces (foo/team/repo, bar/team/repo) both read as team/repo; the error is in the blocking direction.
A worktree attached to a bare repository resolves to the bare repository's parent directory. These workflows do not produce that layout.
The two implementations agree that a directory in place of a branch list is an error; an exotic file type such as a fifo is covered by no row.
cmd/flake-update-cascade also reads protected-branches, and is unchanged. It builds its key as work/<repo> from the registry name and never looks at the checkout, so it cannot notice a misplaced one and misses any entry not written in that form. That deserves its own issue.
protected-branches remains a hand-maintained mirror of the forge's rules.

Validation

Run by the integrator on fba6d3f:

Command	Result
`nix build .#checks.x86_64-linux.go-checks .#checks.x86_64-linux.allod-parity .#checks.x86_64-linux.git-hook-suites`	exit 0
`bash tests/git-hooks/protected-refs-policy.sh`	89 passed
`bash tests/git-hooks/setup-tracked-hooks.sh` (inside `git-hook-suites`)	7 passed
`bash tests/allod-change.sh` against a binary built from this branch	270 passed
`go vet ./...`, `go test -count=1 ./cmd/allod/ ./internal/gitremote/`	clean, ok
Sabotage in a scratch copy: `remote_identity` made to fail	suite red at `case flat-checkout: pre-commit blocked`, "hook allowed the commit"

Reported by the implementing agent and not repeated by the integrator: a red run before each of the six fixes below and a green one after; sabotage of the Go remote match and of the hook's worktree resolution.

Not run: nix flake check as a whole (one check needs a network input, and the other suites are untouched); the deployed hook.

Review

One cold review by gpt-5.6-sol at high effort, of 8c7ce55, followed by one fix round. Seven defect findings (six medium, one low) and twelve comment findings.

Finding	Outcome
Go and the hook chose the repository directory differently (submodule, `--separate-git-dir`)	fixed in `e841cff`
A branch list that cannot be read reads as unprotected	fixed in `fbd81bd`; an absent list stays unprotected
Only the first of several entries for one repository was honoured	fixed in `a6a5b83`
`$HOME` compared as text; an entry's trailing slash	fixed in `ff537ee`
Remote identity compared with case	fixed in `acec5d1`
`awk -v` rewrites backslashes in a path	fixed in `67f67b5`
Comments that narrate	trimmed in `b44fac0`; section dividers in the hook suite were shortened to match the file's existing style, not removed
Worktree of a bare repository	discarded, see Known limits
Nested remote namespaces	discarded, see Known limits
Uppercase `.GIT` suffix	discarded

## Owner summary - **What changes on a running machine:** a protected repository is now recognised by the repository its `origin` names, not only by where it is checked out. Checked out in the wrong place, `allod change begin` and `allod change record` refuse (exit 8) and the git hook blocks commits, pushes, rebases and merges on the protected branch; each message names the expected and the actual path. - **Also changes:** inside a worktree the hook now applies the branch and signing rules of the repository the worktree belongs to, and repo-local hooks run there. A branch list that is present but cannot be read stops the operation instead of reading as "nothing is protected". - **What stays identical:** a repository at its listed path, a repository whose origin matches no entry, a repository with no origin, and a machine with no branch list all behave as before. Work on `agent/*` branches in worktrees is not blocked. - **What proves it:** one table of 43 cases runs against both the Go tool and the bash hook; the hook suite (89), the `allod change` suite (270) and the Go tests pass, and the hook suite goes red when the remote match is disabled. - **Not proven:** the hook as deployed. It reaches a machine through `allod/archetypes`, so nothing changes anywhere until that input is bumped and home-manager rebuilds. After the rebuild, confirm by hand: a commit on `master` from a worktree of a protected repo is blocked, and a commit on an `agent/*` branch there goes through. - **If it is wrong:** revert this PR, or hold the `allod/archetypes` bump. The forge still rejects an agent's push to a protected branch whatever the local rails do. Closes allod/tools#112 ## What was decided **The rule lives twice, held together by one table.** The tool is Go and the hook is a single bash file that `allod/archetypes` copies alone to `~/.config/git/`. Of the three packaging options the issue thread lists, this takes accepted duplication: no change in `allod/archetypes`, no second deployed file, and no dependency of the hook on the `allod` binary. `tests/fixtures/protection-cases.tsv` is read by `cmd/allod/protection_test.go` and by `tests/git-hooks/protected-refs-policy.sh`, so the two cannot drift without a red test. **The rule.** The lookup key is the repository a checkout belongs to: for a linked worktree the main repository, otherwise the checkout itself (which is also right for a submodule and for `--separate-git-dir`). An entry whose path equals that key answers first. Failing that, `owner/repo` is taken from `git remote get-url origin`, and an entry whose path is `owner/repo` or ends in `/owner/repo` means the checkout is misplaced. The remote identity is compared without case, as the forge does; the path match stays exact. **What each rail does with a misplaced checkout.** The tool refuses `begin` and `record` outright, on any branch, since the fix is to move the checkout. The hook blocks exactly what it would block on the protected branch at the right path, and requires signing likewise, and leaves other branches alone. **A new flake check, `git-hook-suites`.** Nothing ran `tests/git-hooks/*.sh` automatically, so the bash half of the shared table would never have been exercised. ## Risk R3. The hook sits on the path of every commit and push in every repository on a dev machine, and the agent cannot exercise it as deployed. The worst credible failure after these checks pass is the hook refusing legitimate work on some repository state no fixture covers, which a human undoes by reverting and rebuilding; the other direction, falling open, leaves the forge's server-side protection in place. No state is destroyed either way. Known limits: - The remote identity is the last two components of the origin URL, as the issue specifies. Two different repositories on a host with nested namespaces (`foo/team/repo`, `bar/team/repo`) both read as `team/repo`; the error is in the blocking direction. - A worktree attached to a bare repository resolves to the bare repository's parent directory. These workflows do not produce that layout. - The two implementations agree that a directory in place of a branch list is an error; an exotic file type such as a fifo is covered by no row. - `cmd/flake-update-cascade` also reads `protected-branches`, and is unchanged. It builds its key as `work/<repo>` from the registry name and never looks at the checkout, so it cannot notice a misplaced one and misses any entry not written in that form. That deserves its own issue. - `protected-branches` remains a hand-maintained mirror of the forge's rules. ## Validation Run by the integrator on `fba6d3f`: | Command | Result | | --- | --- | | `nix build .#checks.x86_64-linux.go-checks .#checks.x86_64-linux.allod-parity .#checks.x86_64-linux.git-hook-suites` | exit 0 | | `bash tests/git-hooks/protected-refs-policy.sh` | 89 passed | | `bash tests/git-hooks/setup-tracked-hooks.sh` (inside `git-hook-suites`) | 7 passed | | `bash tests/allod-change.sh` against a binary built from this branch | 270 passed | | `go vet ./...`, `go test -count=1 ./cmd/allod/ ./internal/gitremote/` | clean, ok | | Sabotage in a scratch copy: `remote_identity` made to fail | suite red at `case flat-checkout: pre-commit blocked`, "hook allowed the commit" | Reported by the implementing agent and not repeated by the integrator: a red run before each of the six fixes below and a green one after; sabotage of the Go remote match and of the hook's worktree resolution. Not run: `nix flake check` as a whole (one check needs a network input, and the other suites are untouched); the deployed hook. ### Review One cold review by `gpt-5.6-sol` at `high` effort, of `8c7ce55`, followed by one fix round. Seven defect findings (six medium, one low) and twelve comment findings. | Finding | Outcome | | --- | --- | | Go and the hook chose the repository directory differently (submodule, `--separate-git-dir`) | fixed in `e841cff` | | A branch list that cannot be read reads as unprotected | fixed in `fbd81bd`; an absent list stays unprotected | | Only the first of several entries for one repository was honoured | fixed in `a6a5b83` | | `$HOME` compared as text; an entry's trailing slash | fixed in `ff537ee` | | Remote identity compared with case | fixed in `acec5d1` | | `awk -v` rewrites backslashes in a path | fixed in `67f67b5` | | Comments that narrate | trimmed in `b44fac0`; section dividers in the hook suite were shortened to match the file's existing style, not removed | | Worktree of a bare repository | discarded, see Known limits | | Nested remote namespaces | discarded, see Known limits | | Uppercase `.GIT` suffix | discarded |

allod-agent added 10 commits

2026-09-27 16:20:56 +01:00

gitremote: add RepoFromURL for a remote URL's owner/repo e51eb22448

The protection rails need the owner/repo of a remote URL they already
hold, for a repository other than the process's own cwd, which InferRepo
cannot give them. RepoFromURL applies the same rule to a URL argument,
with one difference: it strips a trailing slash instead of letting one
defeat the match. InferRepo keeps bash's behaviour there because forge's
callers are pinned to it.

Refs allod/tools#112

Key the protection rails on the repository origin names 8c7ce554f5

A protected repository checked out anywhere other than its
protected-branches entry used to resolve to "unprotected" in both rails,
so a commit on its default branch drew no refusal and no hook block. A
linked worktree did the same for free, whatever the layout: the hook
keyed on the worktree's own toplevel, so every worktree was a rail-free
zone for bare git, for protected-branches and signing-required-branches
alike.

Both rails now resolve the repository first — the main repository's path
relative to $HOME, never a worktree's own — and, when no entry names that
path, ask what origin names. An entry matches an origin identity when its
path is the remote's owner/repo or ends in /owner/repo. A positive remote
match with no path match is a misplaced checkout, not an absence of
protection:

- 'change begin' and 'change record' refuse it outright, on every branch,
  with exit 8, naming the repository, the expected path and the actual one.
- the hook blocks that entry's branch where it would on a path match,
  pre-commit, pre-rebase, pre-merge-commit and pre-push, and names both
  paths beside the branch.

A repository with no origin, or one whose origin matches no entry, stays
unprotected and silent.

run_repo_hook resolves .git/hooks through the common git dir, so a
repo-local hook runs from a worktree too. Not 'git rev-parse --git-path
hooks', which returns core.hooksPath when it is set — this script lives
there, so it would call itself.

The rule is implemented twice, in Go and in bash, because the hook is
deployed as a standalone file with no access to the allod binary.
tests/fixtures/protection-cases.tsv is what keeps the two from drifting:
the Go test and the hook suite both answer to it. The new git-hook-suites
flake check runs the hook suite, which nothing ran before, so the bash
half of that guard is exercised too.

Refs allod/tools#112

Resolve the repository the same way in Go as in the hook e841cffcb3

mainRepoDir always took the parent of --git-common-dir, which is the
repository only in the ordinary layout. A populated submodule keeps its
common dir under the superproject's .git/modules, and
git init --separate-git-dir puts it anywhere, so Go looked up a directory
that is not a checkout at all: it permitted change record on a listed
submodule's protected branch, and classified a correctly placed
--separate-git-dir checkout as misplaced. The hook was already right,
because --git-dir equals --git-common-dir in both layouts.

Go now uses that same test: the parent of the common dir for a linked
worktree, --show-toplevel otherwise.

The shared table gains a 'layout' column (plain, worktree, submodule,
separate-git-dir) and a 'home' column, with rows for the two new layouts;
tests/allod-change.sh gains a submodule and a separate-git-dir fixture at
the rail level.

Refs allod/tools#112

Fail loudly on a branch list that cannot be read fbd81bd6ae

An absent list stays what it was: no policy on this machine, silently
unprotected. A list that is present and unreadable used to reach the same
verdict, so a directory in place of ~/.config/git/protected-branches
disarmed both rails.

Go opened the directory successfully, ignored the scanner's error and
returned an empty list; readBranchList now returns an error, which the rails
turn into a refusal naming the file. It also raises the scanner's line limit
and reports a line over it, because bufio.Scanner's 64 KiB default silently
ends the scan and every entry behind a long line would go unseen while the
hook's awk read them.

The hook cannot learn this from awk: gawk skips an unreadable or non-regular
argument with a warning and still exits on its END status, so the file is
tested directly. awk's own status is checked as well, above 1 meaning a read
failure rather than "not found", at each of the three call sites, since all
of them sit where errexit is suspended.

Refs allod/tools#112

Treat every entry for a repository as listed, not only the first a6a5b83625

protected-branches allows one line per protected branch, and a repository
with two took only its first line in Go, so 'change record' committed and
pushed from the second listed branch while the hook blocked it. On a
misplaced checkout both implementations stopped at the first remote-matching
line, so neither blocked nor required signing for any branch but that one.

protection now carries every branch its matching entries name: covers()
answers the record refusal and the hook's block, start() the branch begin
branches from, which is still the first listed. The hook's remote-match awk
scans past a non-matching branch instead of stopping at the first entry.

signing-required-branches goes through the same function, so it gains the
same behaviour; a hook case pushes to the second of two signing entries.

Refs allod/tools#112

Compare $HOME as a resolved path, and ignore an entry's trailing slash ff537ee5db

git reports physical paths. Both rails stripped the unresolved $HOME from
them, so on a machine whose $HOME is a symlink every correctly placed
protected repository read as misplaced and 'change record' refused even an
agent/* branch. A $HOME with a trailing slash did the same to the hook alone,
which is the shape of divergence this table exists to catch.

Each implementation now resolves $HOME once, before stripping: Go through
EvalSymlinks, the hook through 'cd && pwd -P', both falling back to the
literal value with trailing slashes removed when it cannot be resolved.

An entry path's trailing slash is dropped as well. 'work/acme/widget/' names
the same directory as 'work/acme/widget' but matched neither the checkout
path nor the remote suffix, so such an entry protected nothing at all; the
normalised form is what the refusal reports.

Refs allod/tools#112

Match the remote identity without case acec5d1456

The forge treats owner and repository names case-insensitively, so origin
Acme/Widget is the repository an entry work/acme/widget names. Both
implementations compared the identity against entry tails as-is, so a
misplaced checkout whose origin differed only in case fell through to
"unprotected".

Only the remote match ignores case. The path match stays exact, because a
path is a path: a case row pins that too.

Refs allod/tools#112

Hand awk its values through the environment, not -v 67f67b5160

awk expands escape sequences in a -v assignment, so a checkout path or branch
holding a backslash reached the comparison with it removed: a canonical
checkout at work/acme\widget matched neither its own entry nor the remote
suffix, and the hook permitted its protected branch while Go protected it.

Every path, identity and branch now goes to awk through ENVIRON.

Refs allod/tools#112

Cut the comments that narrate instead of warning b44fac0ce5

Every comment the review named, plus the ones this round added, against one
test: a contract the code cannot express, the trap the obvious alternative
falls into, or a pointer. Function summaries that restate a name, positional
phrases like "the cases below", and the paragraph in the fixture explaining
why two implementations exist are gone; the fixture keeps its column schema
and what each verdict means.

The pointers tying each twin to the fixture stay, one or two lines each.
Section dividers in the hook suite keep the one-line form the rest of that
file already uses, with the prose under them removed.

Refs allod/tools#112

git-hooks/README: match what the rule now does fba6d3f5c1

The recognition section predates this round: it described one entry per
repository, an unresolved $HOME, a case-sensitive remote match, and said
nothing about a list that cannot be read or about the two layouts whose git
directory sits away from the worktree.

Refs allod/tools#112