Fourth PR-review round on #948. All four reproduced first.
1. The gate was one flag away from opt-out. --waive applied to whatever
gate_refusals() returned, including G1 "no review round has been collected",
so `close --waive "no time"` exited 0 with nobody having looked at the
artifact. That is the most tempting shortcut for an agent under time
pressure and it defeats the skill's whole premise.
G1 is now unwaivable, with its own refusal message: a waiver accepts
objections a reviewer raised, it cannot manufacture a review that never
happened. Waiving a genuine objection (G2/G3/G4/G7) still works.
2. Inline `style` was unsanitized, so a reviewed draft containing
`background-image:url(https://attacker/beacon.png)` fired a request the
moment the reviewer opened the page — no script needed, and directly
contrary to the no-network property the README and manifest advertise.
Added to DROP_ATTRS. The <style> tag was already dropped, so keeping the
attribute was inconsistent as well as leaky.
3. Markdown `` never rendered. LINK's regex was not anchored against
a preceding `!`, so an image became `!<a href=...>` — and _safe_href's
image=True branch, which exists to allowlist data:image URIs, was dead code
on that path. Added an IMAGE regex ahead of LINK, negative-lookbehind on
LINK, and real <img> rendering through the same scheme allowlist. Verified a
javascript: src degrades to inert alt text.
4. An unterminated <script> silently swallowed the rest of the body — same
confusing failure shape as the void-tag bug, though it fails safe. Now emits
a named diagnostic to stderr instead of vanishing.
Nit: raw-HTML `target="_blank"` anchors get the rel="noreferrer noopener" the
Markdown path already added to its own.
Re-verified: derive_counters --check, check_plugin_json --all, checklist 6/6
PASS, description validator PASS, all three scripts --help/--sample green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01233Eggb2cjSYf96X6C3pCm
Third PR-review round on #948. All three reproduced first.
1. HIGH — every realistic HTML5 document produced zero blocks. meta, link and
base are void elements: html.parser fires handle_starttag for them but never
a matching handle_endtag. They were also in DROP_TAGS, so each bare
`<meta charset>` incremented self._skip permanently and every subsequent
starttag/endtag/data callback inside <body> hit the skip guard. Result:
empty out, empty blocks, "No reviewable blocks found", exit 2.
The documented landing-page use case therefore did not work at all. It
survived three review rounds because every HTML fixture I wrote used only
<title>/<style> in head — the sanitizer test included. Void drop-tags no
longer touch the counter.
--sample now builds BOTH fixtures (Markdown + a full DOCTYPE HTML5 doc with
bare meta/link), asserts the expected block count for each, and exits 2 on
regression, so this cannot come back silently. It also writes to a temp dir
instead of cwd — the same class of mistake that leaked a stray artifact into
an earlier commit.
2. MEDIUM — xlink:href bypassed the URL allowlist. SVG anchors still honour it,
so `<svg><a xlink:href="javascript:alert(1)">` survived the hardening added
one commit earlier. Added with xlink:role and xlink:arcrole.
3. MEDIUM — status did not mirror close. It inspected only blocking_open, so a
round with no named reviewer reported exit 0 while close refused on G3 —
directly contradicting the exit-code contract the docstring advertises.
Both now call a shared gate_refusals(), so they cannot drift: verified they
agree on 2 (G3 open) and on 0 (clean round). status also prints which rules
would refuse rather than just a count.
Re-verified: derive_counters --check, check_plugin_json --all, checklist 6/6
PASS, description validator PASS, all three scripts --help/--sample green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01233Eggb2cjSYf96X6C3pCm
Second PR-review round on #948. All four reproduced before fixing.
1. HIGH — reviewed HTML executed in the review page. BlockTagger re-emitted
attributes verbatim, escaping values but never filtering attribute names or
URL schemes. The Markdown path has had _safe_href scheme-allowlisting all
along; the HTML path had nothing. Reproduced: a draft.html containing
`<img src=x onerror=...>`, `<a href="javascript:alert(1)">` and an <iframe>
passed straight into the page a reviewer opens — and reviewing a landing-page
draft is a documented use of this skill.
sanitize_attrs() drops on* handlers, srcdoc and srcset, and runs href/src/
action/formaction/poster/cite/background through the scheme allowlist.
_safe_href now strips control characters before reading the scheme (so
`java\tscript:` cannot smuggle one) and allows data:image only for image
attributes. DROP_TAGS removes iframe/object/embed/frame/base/applet as well
as script/style/head/link/meta. Verified: handlers, javascript: (plain and
tab-smuggled), and iframes all gone; https links and relative images kept.
2. MEDIUM — verify_quotes compared rendered text against raw markup. A quote
comes from window.getSelection(), which is what the browser rendered, so
selecting a sentence containing **bold**, `code` or a link never matched the
raw source. G7 had just made that blocking, so this refused legitimate
closes. Now matched against raw OR a rendered-text projection (inline markup
stripped for Markdown, tags stripped and entities unescaped for HTML). A
fabricated quote is still caught — verified both directions.
3. MEDIUM — state["waiver"] was never cleared, so after waived-close → reopen →
a clean round, close still printed the old waiver reason. For a tool whose
premise is an honest record of what was actually reviewed and waived, that is
its own integrity bug. Cleared whenever a close passes with zero refusals.
4. LOW — status returned 4 for both "no sidecar yet" and "collected, blockers
open". The blocked case now returns 2, matching close, so an agent can branch
on the exit code alone: 0 clear, 2 blocked, 3 collect, 4 nothing yet.
Also corrected an over-broad claim of my own: the page makes no network request
of its own, but a reviewed HTML artifact's own https: assets do load, as they
must for the review to be faithful. README and SKILL.md now say that precisely.
Re-verified: derive_counters --check, check_plugin_json --all, checklist 6/6
PASS, description validator PASS, all three scripts --help/--sample green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01233Eggb2cjSYf96X6C3pCm
Both from PR review on #948, both reproduced before fixing.
G7 — a real hole in the skill's core promise. feedback_parser downgrades an
unrecognised severity heading to NIT and records it only as advisory prose in
`problems`, which cmd_close never read. Reproduced: a sidecar with `## BLOKCER`
carrying "No source. Do not ship this." collected as a NIT, and close exited 0 —
a reviewer's genuine blocker lost to a typo. Same gap covered EDIT items with no
`+ after:` line and quotes that do not appear in the target file.
close now refuses (exit 2) while the last collected round carries unresolved
integrity problems. Problems that already have their own rule are filtered via
GATED_ELSEWHERE so G2/G3 are not double-reported. Verified: typo'd severity,
missing EDIT replacement, and quote-not-in-file each refuse; a clean sidecar
still passes; a missing reviewer still reports G3 alone.
Stray artifact — quarterly-plan.review.html was committed at the repo root. It
came from a `review_page_builder.py --sample` run during the post-merge
verification sweep with cwd at the repo root, then got swept up by `git add -A`.
Removed, and .gitignore now covers `*.review.html` + `.human-gate/` so neither
this repo nor a user of the skill re-commits a disposable review page. The
sidecar (<artifact>.review.md) is deliberately NOT ignored — that is the
reviewer's feedback and belongs in git.
G7 documented in the script docstring, SKILL.md, README, the command, plugin.json
(description + derivation_note) and CHANGELOG. Not acted on: the reviewer's note
that cmd_status's success line is terse — they flagged it as "not a real issue"
and the JSON branch already carries blocking_open.
Re-verified: derive_counters --check passes, check_plugin_json --all 90/90,
write-a-skill checklist 6/6 PASS, description validator PASS, all three scripts
--help/--sample green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01233Eggb2cjSYf96X6C3pCm
Audits petergyang/human-review and ships a conceptual derivation that fits this
repo's stdlib-only conventions.
Audit (audit/human-review-2026-08/AUDIT.md): upstream is a well-engineered ~5,200
LOC Node app — its own test suite passes 90/90, and its security model (loopback
bind, DNS-rebinding Host check, constant-time token compare, realpath traversal
guard, inert Markdown renderer, 45-min idle shutdown) is better than most
local-server tools. It still does not fit: Node 20 + an npm runtime dependency
fails the same stdlib-only test that kept the heavier skillopt package out in
v2.11.2. Seven findings, three material — F1 (HIGH) unpinned `npx -y` executes a
newly published version on every run; F2 (MED) "do not end your turn" plus
re-poll on timeout with no headless guard or retry cap; F3 (MED) only /api/* is
token-gated.
Also: despite the name it is not a humanizer. This is human approval, not human
voice — no overlap with behuman or content-humanizer.
New plugin engineering/human-gate, three stdlib scripts, no server or socket:
- review_page_builder.py — Markdown/HTML to a single-file anchored review page
with zero network requests (~11 KB, opens over file://). Escapes before
applying inline markup, scheme-allowlists hrefs, drops script/style on HTML
input.
- feedback_parser.py — sidecar to batch.v1 JSON. BLOCKER/MAJOR/MINOR/NIT
(matching md-review) plus EDIT/NOTE/APPROVE. Verifies quotes against the real
file; strips HTML comments so a documented example cannot parse as a real
sign-off.
- human_gate.py — open/status/collect/close/reset with atomic writes and
0700/0600 state. Rules G1-G6 refuse to close on: no collected round, an open
BLOCKER/MAJOR, an unnamed reviewer, a sidecar changed after collection, an
exhausted round cap (exit 5 = escalate), or an undocumented waiver.
Loop discipline deliberately inverts upstream: no blocking poll, a headless
guard, a round cap that escalates. The sidecar is hand-writable Markdown, so the
loop closes over SSH and in CI. The optional bridge to upstream is opt-in and
always version-pinned.
Adds 3 references (7-8 sources each), a batch.v1 schema, a worked example,
cs-human-gate agent, /cs:human-gate command. SKILL.md passes the write-a-skill
6-item checklist 6/6; description validator PASS.
Counters: skills 362->363, tools 644->647, refs 741->744, agents 102->103,
commands 116->117, plugins 88->89.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01233Eggb2cjSYf96X6C3pCm
