Security fix: publish spec-size fixes through staged text

[JacobStephens2]

Summary

Spec #646 publishes spec-size Security fixes through staged text validated
before any public issue exists. Its Tickets both landed on issue-646:
#647 (stage + validate the Spec proposal, thirdshift creates the issues and
their native links) and #648 (pre- and post-publish window sweeps that close
and loudly fail on any session-created side-effect issue). The code review
added one fail-closed fix on top: staged Blocked by edges must name earlier
Tickets only.

publish (public, spec-size)
  pre-creation window sweep → fail_on_side_effects on any leak
  parse_staged_spec_proposal (all text validated before anything is created)
    parse_blocked_by (only earlier Tickets; forward/cyclic edges rejected)
  create top issue + Tickets
  add_sub_issue + add_blocked_by links
  post-linking window sweep (unlinked_issues) → fail_on_side_effects on any leak
  post-publish checks (open, new, labels, text)

Evidence

• Before: the review's cyclic-spec probe
  (a_staged_spec_with_cyclic_blocking_edges_is_rejected_before_creating_anything)
  FAILED on the pre-fix code — a 1↔2 mutual-block staged spec published
  issues 8, 9 and 10 instead of being rejected:
  a rejected staged spec must not create a public issue: {"10":..., "8":..., "9":...}.
  After: the test passes; staged forward/cyclic edges are rejected before
  thirdshift creates anything.
• Focused runs: 5/5 staged_spec* unit tests pass (incl. the new forward-edge
  case in staged_spec_rejects_bad_blocking_edges), 1/1 new integration test
  passes, cargo fmt --check passes,
  cargo clippy --all-targets -- -D warnings passes.
• Full cargo test: exit 0, every target ok, 0 failed.

Seams under test: the secure security-fix staged-spec publish path
(integration seam, tests/security_run.rs) and parse_staged_spec_proposal /
parse_blocked_by (unit seam, src/security/fixing.rs).

Merge Danger

Door: two-way — a revert restores the previous publish path; validation now
rejects bad staged text before any public issue exists, so there is nothing
half-published to unwind from the fix itself.

Blast Radius: security-fix publishing — public spec-size path only. The
single-fix staged path and the private session-created path are untouched
(the ordering guard only narrows what staged text validates).

Unaddressed findings

Each Standards finding or Spec finding the review chose not to fix, with what
refutes it. No needs-triage issue was filed: every decline below is the
review's own engineering call, not a Spec or Day shift decision, and no open
issue already covers any of them.

• Standards S1 (Duplicated Code, judgement): api_add_sub_issue and
  api_add_blocked_by in tests/fakes/gh.rs:724 / :758 share a shape.
  Declined: each mirrors a distinct GitHub endpoint (sub_issues vs
  dependencies/blocked_by); extracting one helper would couple two adapters
  that change for different upstream reasons.
• Standards S2 (Duplicated Code, judgement): parse_staged_proposal
  (src/security/fixing.rs:381) and parse_staged_spec_proposal (:209)
  share a head-parsing shape. Declined: the spec head carries ticket-trailer
  grammar the single path lacks; sharing a helper would tangle two formats
  that fail for different reasons.
• Standards S3 (Primitive Obsession, weak judgement): blocked_by: Vec<usize>
  (src/security/fixing.rs:192). Declined: repo idiom overrides — u64 /
  usize / &str are pervasive across src/github.rs for issue identity.
• Spec 1 (blocking links never read back against the native graph): the code
  fact holds — test -n "$(grep -n 'blockers' src/security/fixing.rs)"
  exits 1. Declined as a fix: add_blocked_by errors propagate via ?
  (src/security/fixing.rs:327-329), so a failed edge aborts publishing
  rather than dispatching silently — there is no silent-drop path to catch.
  Neither Stage spec proposal text and validate before creating public issues #647 ("validate it before creating any public issues") nor Sweep spec publish session for unlinked public issues #648
  (window sweep) asks for native-graph read-back, and the pre-existing private
  path never verified blockers either; a read-back would be new behaviour
  beyond the Spec. Adjacent but distinct: Decide staged-issue fate when the post-linking spec sweep fails #655 covers staged-issue fate when
  the post-linking sweep fails, not link verification.
• Security (no introduced findings, no fix): reviewed the staged-spec publish path against the skill's data-isolation and agent-output guidance: publish_staged_spec / parse_staged_spec_proposal / parse_blocked_by (src/security/fixing.rs), add_sub_issue / add_blocked_by / post_by_id (src/github.rs), and the security-fix prompt. All staged text is validated with check_public_fix_text before any public issue exists; pre-creation and post-linking window sweeps close side-effect issues and fail loudly; forward/cyclic Blocked by edges are rejected. Evidence: 5/5 staged_spec* unit tests pass; 8/8 spec-path security_run integration tests pass, including the cyclic-rejection probe and the side-effect-leak probe. No proof-of-concept demonstrated a boundary violation, so this review changed no source, added no test, and made no commit.

Review coverage: the Standards and Spec reviewers each read all 7 changed
files (prompts/security-fix.md, site/prompts/index.html, src/github.rs,
src/prompt.rs, src/security/fixing.rs, tests/fakes/gh.rs,
tests/security_run.rs). No changed file left unread.

Full per-axis reports: .thirdshift-review-uSwFnb/standards.md and
.thirdshift-review-uSwFnb/spec.md (scratch, git-excluded).

Closes #646

Security review outcome

No unaddressed introduced findings.

Tickets

• Stage spec proposal text and validate before creating public issues #647 landed with Stage spec proposal text and validate before creating public issues #650
• Sweep spec publish session for unlinked public issues #648 landed with Sweep spec publish session for unlinked public issues #654

Built with muse · default model · default effort