Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/validate-task.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,15 @@ jobs:
# Each gate in scripts/ is proven by a case that reintroduces the fault it catches.
# Standard library only, so no install step and no dependency to pin.
# The audit engine self-test is offline, so it runs here rather than only on an owner sweep.
# The write-guard self-test is offline too, and it otherwise runs only when a host installs the hook, which is where a regression in it would surface as a broken machine.
- name: Run script self-tests step
run: |
set -Eeuo pipefail
python3 scripts/test_prose_lint.py
python3 scripts/test_repo_gate.py
python3 scripts/test_pr_review.py
python3 spec/audit.py --selftest
python3 host-setup/agent-safety/gh-write-guard.py --selftest

- name: Check repo gates step
run: python3 scripts/repo_gate.py
Expand Down
1 change: 1 addition & 0 deletions OPERATIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ python3 scripts/test_prose_lint.py
python3 scripts/test_repo_gate.py
python3 scripts/test_pr_review.py
python3 spec/audit.py --selftest
python3 host-setup/agent-safety/gh-write-guard.py --selftest
python3 scripts/repo_gate.py
python3 scripts/prose_lint.py . --check charset --check dupword --check spelling
python3 scripts/prose_lint.py . --check charset-unknown --check semicolon --check dash --check comment-wrap --check comment-case --summary
Expand Down
17 changes: 1 addition & 16 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,21 +62,6 @@ One pull request adding two gates of the same shape, each confirming that a refe
- **Settled** - Three stale descriptions in one session generated six review findings between them, each a reviewer noticing that the body named a commit, a branch, or a behavior the branch no longer carried.
- **Settled** - Prose claims stay out of scope, since judging those needs a similarity heuristic, which [`spec/section-model.md`][section-model] rejects for the reason it would fail here.

### The Write-Guard Newline Defect

One pull request fixing the argument-list split in the installed hook, plus the self-test case that locks it.

**State** `ready`. **Touches** [`host-setup/agent-safety/gh-write-guard.py`][write-guard] and its self-tests. **Cost** one hub edit, and it reaches a machine only when the installer is re-run, so it rides the host visit in "Fleet Sweeps".

- **End a `git push` argument list at a newline, not only at `&&`.** Every token on a later line of the same command is otherwise read as a refspec.
- **Blocked by** - Nothing.
- **Issue** - None filed, and [#365][issue-365] carries the rollout that delivers it.
- **Checked** - `develop` at `1ed0cc8` on 2026-08-03, measured against the installed hook.
- **Open** - Nothing.
- **Settled** - A lone `git push -u origin revendor/x` resolves to the one branch, while the same push followed by a newline and a `gh pr create` with a base of `develop` resolves to five, meaning `revendor/x`, `gh`, `pr`, `create`, and `develop`.
- **Settled** - The `&&` form resolves correctly, which is what isolates the defect to the newline case, and the existing suite covers only that form.
- **Settled** - The error direction is over-blocking rather than under-blocking, so it is a usability defect rather than a safety hole, and that is why it is worth fixing: the denial claims a direct push to a protected branch when the push targets an ordinary feature branch, and teaching a safety hook to cry wolf is how it stops being read.

### Three Rules That Leave the Recurring Case Unstated

One pull request widening three carried [`GOVERNANCE.md`][governance] rules that each state their common case and go quiet on the case that recurs, filed together because they share that shape and land in one re-vendor.
Expand Down Expand Up @@ -428,7 +413,7 @@ Regenerate [reports/divergences.md][divergences-report] before using it as the w
- **Hub state** - Done for the documentary half, verified `develop` at `1ed0cc8` on 2026-08-03.
- **Outstanding** - Four machines, WSL2 Ubuntu, the MacBook Air and both ThinkPads, plus any headless or cron environment running with the token. macOS needs someone on that platform, the Proxmox question is whether that host also runs containers which decides whether Docker is required there, and the engine-inside-the-distro variant of the WSL2 Docker cell is unverified.
- **Issue** - [#365][issue-365] and [#483][issue-483].
- **Rides with** - The write-guard newline fix, since only re-running the installer deploys it.
- **Rides with** - Nothing on the hub, since the write-guard newline fix has landed on `develop` and a machine keeps running the old hook until the installer is re-run there.
- **Detail** - A ticked row means the host-wide rules text and not the hook, since only running the installer deploys both layers, and the proxmox host proved that distinction by carrying the documentary half alone for eight days on the machine where the incident originated.
- **Detail** - Honor the issue's own rule when filling a cell, that an unverified install command is worse than a blank, because a blank prompts a question while a wrong command produces a broken host and a false sense that setup succeeded.
- **Detail** - The superseded safety section from [#364][issue-364] still sits above the canonical block in this host's rules file, so the two overlap. Removing it is a judgment call on a per-machine file, which is why it is surfaced rather than applied.
Expand Down
47 changes: 35 additions & 12 deletions host-setup/agent-safety/gh-write-guard.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,23 +199,36 @@ def _current_push_branch(cwd):
_GIT_GLOBAL_VALUE_OPTS = {"-C", "-c", "--git-dir", "--work-tree", "--namespace", "--exec-path", "--config-env"}


_SHELL_OP_CHARS = set("();<>|&")
# A newline ends a command exactly as `;` does, so it is an operator character here rather than whitespace.
# Read as whitespace it vanishes when tokenizing, and every token on a later line of a multi-line command is then read as one more argument of the first line's command.
# A backslash-newline continuation is folded to a space in `classify` before any of this runs, so every newline reaching the tokenizer is a real command separator.
# The string is the form shlex takes the set in, and the set is derived from it so the two cannot drift apart.
_PUNCTUATION_CHARS = "();<>|&\n"
_SHELL_OP_CHARS = set(_PUNCTUATION_CHARS)


def _shell_tokens(cmd):
"""Tokenize like a shell, isolating operator runs (`|`, `&&`, `;`, `>`, `2>&1`, ...) as their own
tokens even when glued to a word - so a `>` inside a quoted value stays part of that token while a
real redirection is separated. Degrades gracefully if the quoting cannot be parsed.
"""Tokenize like a shell, isolating operator runs (`|`, `&&`, `;`, newline, `>`, `2>&1`, ...) as
their own tokens even when glued to a word - so a `>` or a newline inside a quoted value stays part
of that token while a real redirection or line break is separated. Degrades gracefully if the
quoting cannot be parsed.
"""
try:
lex = shlex.shlex(cmd, posix=True, punctuation_chars=True)
lex = shlex.shlex(cmd, posix=True, punctuation_chars=_PUNCTUATION_CHARS)
lex.whitespace_split = True
lex.whitespace = lex.whitespace.replace("\n", "") # A newline is an operator above rather than a gap between words.
return list(lex)
except (ValueError, TypeError): # bad quoting, or punctuation_chars unsupported on old Python
try:
return shlex.split(cmd, posix=True)
except ValueError:
return cmd.split()
# Neither fallback isolates an operator, so the lines are split here to keep the one thing this path must not lose, that a newline ends the command before it.
toks = []
for i, line in enumerate(cmd.split("\n")):
if i:
toks.append("\n")
try:
toks.extend(shlex.split(line, posix=True))
except ValueError:
toks.extend(line.split())
return toks
Comment thread
ptr727 marked this conversation as resolved.


def _is_shell_op(tok):
Expand All @@ -227,7 +240,7 @@ def _is_redir_op(tok):


def _is_separator(tok):
return _is_shell_op(tok) and ">" not in tok and "<" not in tok # |, ||, &, &&, ;, (, )
return _is_shell_op(tok) and ">" not in tok and "<" not in tok # |, ||, &, &&, ;, (, ), newline


def _is_git_exe(tok):
Expand All @@ -243,7 +256,8 @@ def _git_subcommand_arglists(cmd, sub):

Keying off a real `git`->`<sub>` token sequence (git's value-taking global options skipped, an
absolute-path or .exe git recognized) means the same invocation named inside a quoted --body forms no
such sequence, and a compound `<sub> A && <sub> B` yields two independent arg lists so both are seen.
such sequence, and a compound `<sub> A && <sub> B` yields two independent arg lists so both are seen,
whether the two are joined by `&&` or written on their own lines.
"""
toks = _shell_tokens(cmd)
n = len(toks)
Expand All @@ -265,7 +279,7 @@ def _git_subcommand_arglists(cmd, sub):
while k < n:
t = toks[k]
if _is_separator(t):
break # a command separator (|, &&, ;) ends this git invocation
break # a command separator (|, &&, ;, newline) ends this git invocation
if t.isdigit() and k + 1 < n and _is_redir_op(toks[k + 1]):
k += 1 # a file-descriptor number before a redirection is shell syntax, not git argv
continue
Expand Down Expand Up @@ -622,6 +636,15 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None,
("gh issue comment 5 --body \"first git push\" && git push origin develop", None, {"develop": _CODE_RULES}, "deny", "a quoted mention before a real push does not hide the real target"),
("git push >push.log 2>&1", "develop", {"develop": _CODE_RULES}, "deny", "redirection tokens are not a branch: bare push to develop still denies"),
("git push origin develop >push.log 2>&1", None, {"develop": _CODE_RULES}, "deny", "redirect after a real refspec does not hide the develop target"),
# A newline ends a command as `&&` does, and reading it as whitespace made every token on a later line an argument of the push.
# A feature-branch push followed by a `gh pr create` then denied as a direct push to the base branch that command named.
("git push -u origin feature/x\ngh pr create --base develop --title x --body y", None, {"feature/x": set(), "develop": _CODE_RULES}, "allow", "a newline ends the push argv: the pr-create base is not a push target"),
("cd /repo\ngit push origin develop", None, {"develop": _CODE_RULES}, "deny", "a push on a later line is still parsed as a push"),
("git push origin feature/x\ngit push origin develop", None, {"feature/x": set(), "develop": _CODE_RULES}, "deny", "a second push on the next line is checked: develop denies"),
("git push \\\n origin develop", None, {"develop": _CODE_RULES}, "deny", "a backslash-newline is a continuation, not a separator: develop still parsed"),
("gh issue comment 5 --body \"one line\ngit push origin develop\"", None, {"develop": _CODE_RULES}, "allow", "a newline inside a quoted body does not start a new command"),
# Unbalanced quoting is what actually reaches the degraded path, and the separator has to survive there too.
("git push origin feature/x\ngit push origin develop 'unclosed", None, {"feature/x": set(), "develop": _CODE_RULES}, "deny", "the degraded path keeps the newline: a push on the next line is still read"),
]


Expand Down
Loading