docs+tools(devsecops): retrospective v0.1 + runaway-timeout / single-copy / hermeticity guards - #1123
docs+tools(devsecops): retrospective v0.1 + runaway-timeout / single-copy / hermeticity guards#1123mdheller wants to merge 1 commit into
Conversation
…copy / hermeticity guards Persist the session retrospective (7 failure classes A-G, config-reoptimization recommendations, and the failures->recurring-detectors feedback loop) and ship three self-contained mechanical guards, each with a red-then-green selftest: - tools/check_workflow_timeout_bounds.py (class C): fails on any GitHub Actions job with no timeout-minutes (inherits the 360m/6h default that burned the Actions budget). Measured on origin/main: 122 of 123 jobs unbounded. - tools/detect_unpushed_single_copy.py (class D): reports commits/stashes/ worktrees that exist on no remote, before a disk-zero erases single-copy work. - tools/check_test_hermeticity.py (class B): AST gate; fails if importable code writes under real $HOME without an env redirect (generalises Noetica #585). New files only; no contested file edited. Local evidence only (Actions spend-capped).
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a DevSecOps retrospective document and three standalone “recovery/stability guard” tools (each with a direct-run selftest) to detect: unbounded GitHub Actions job timeouts, unpushed/single-copy git work, and non-hermetic writes to real $HOME.
Changes:
- Add
tools/check_workflow_timeout_bounds.py+ selftest to audit workflow jobs fortimeout-minutesbounds (with reusable-workflow exemption and fail-closed parsing). - Add
tools/detect_unpushed_single_copy.py+ selftest to flag unpushed commits, stashes, dirty worktrees, and no-remote repos under given roots. - Add
tools/check_test_hermeticity.py+ selftest to statically detect non-redirectable write sinks under real$HOMEin Python code, plus a retrospective design doc capturing failure taxonomy and guard proof obligations.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/check_workflow_timeout_bounds.py | New YAML-based auditor for missing/invalid timeout-minutes per job (with JSON output option). |
| tools/selftest_check_workflow_timeout_bounds.py | Direct-run “red then green” fixtures for the timeout auditor. |
| tools/detect_unpushed_single_copy.py | New local operator scanner for single-copy git state (unpushed/stash/dirty/no-remote). |
| tools/selftest_detect_unpushed_single_copy.py | Direct-run lifecycle selftest for the single-copy detector. |
| tools/check_test_hermeticity.py | New AST-based scanner for non-redirectable write paths derived from real $HOME. |
| tools/selftest_check_test_hermeticity.py | Direct-run selftest suite with positive + negative controls for hermeticity scanner. |
| docs/architecture/devsecops-retrospective-and-recovery-v0.1.md | New retrospective document describing failure classes and shipped/scoped guards. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| print(f"[{status}] {name}: rc={cp.returncode} (want {expect_rc}), " | ||
| f"substr {'found' if expect_substr in (cp.stdout+cp.stderr) else 'MISSING'!r}={expect_substr!r}") | ||
| if not ok: |
There was a problem hiding this comment.
Rejecting this one as a false positive. {'found' if <cond> else 'MISSING'!r} is valid: the !r conversion applies to the whole conditional expression, not to the 'MISSING' literal. Confirmed it compiles under both Python 3.11 and 3.12, and the selftest runs clean (7/7 cases passed). No change needed.
| * The **policy** -- *which* HOME subdirectories count as sacred operator state | ||
| (`~/.noetica`, `~/.config/...`, `~/.ssh`) -- is per-repo. v0.1 takes the | ||
| strict stance that ANY non-redirectable write under real HOME is a violation | ||
| (the safest default); `--allow-subpath` can whitelist specific dirs a repo |
There was a problem hiding this comment.
Valid, minor. The module docstring here implies --allow-subpath exists while argparse doesn't define it. Worth trimming to match the current strict/no-whitelist behavior (the retrospective doc already says "Scoped, not built"). Non-blocking; flagging for a follow-up.
| prophet-platform's 416 `tools/` modules. The **policy** — *which* `$HOME` | ||
| subdirectories count as sacred operator state — is per-repo; v0.1 takes the strict | ||
| default (any non-redirectable real-`$HOME` write is a violation) with an | ||
| `--allow-subpath` escape hatch. **Scoped, not built:** true *test-reachability* |
There was a problem hiding this comment.
Rejecting — the doc doesn't over-promise. This section reads "--allow-subpath escape hatch. Scoped, not built:", i.e. it explicitly states the flag is not implemented, which matches the tool. No mismatch to fix here. (The tool's own module docstring at check_test_hermeticity.py:36 does imply a working flag — that one's a fair trim; see the sibling thread.)
| if do_fetch and has_remote: | ||
| git(repo, "fetch", "--all", "--quiet") | ||
|
|
| # sqlite3.connect(path) creates the file | ||
| if full.endswith("sqlite3.connect"): | ||
| if node.args and target_is_home(node.args[0], home_names): | ||
| out.append(Violation(path, line, "sqlite3.connect()", "db path derived from real $HOME")) | ||
| continue | ||
| # shutil.copy*/move(src, dst) -> dst is the write | ||
| if attr in SHUTIL_WRITE and full.endswith(f"shutil.{attr}"): | ||
| if len(node.args) >= 2 and target_is_home(node.args[1], home_names): | ||
| out.append(Violation(path, line, f"shutil.{attr}()", "destination derived from real $HOME")) | ||
| continue |
|
|
||
| import argparse | ||
| import ast | ||
| import sys |
|
|
||
| import argparse | ||
| import json | ||
| import sys |
|
|
||
| import argparse | ||
| import subprocess | ||
| import sys |
What
Persists the session DevSecOps retrospective and ships three self-contained
stability/recovery guards. Extends the Wave-0 DevSecOps design (receipts on the
contracts/EvidenceReceipt.v0.1.jsonspine; meta-control = the Control TeethRegister). New files only — no contested file edited.
Doc:
docs/architecture/devsecops-retrospective-and-recovery-v0.1.md— the sevenfailure classes (A controls-that-can't-fail · B substrate-ate-itself · C budget-as-
failure · D single-copy recovery · E resume-from-intent · F verify-command-not-
artifact · G instruments-lie), each with *what happened / root cause / program change
mistakes. Plus two program sections: config reoptimization (CLAUDE.md rules,
settings.jsonhooks, permission-allowlist — proposed, not applied) and thefeedback loop that turns the taxonomy into recurring Control-Teeth detectors.
Guards (each with a red-then-green selftest)
tools/check_workflow_timeout_bounds.pytimeout-minutes→ count 122→121, job leaves the list.tools/detect_unpushed_single_copy.pytools/check_test_hermeticity.py$HOMEwrite patterns flagged; env-redirectable/tmp/read stay green. Generalises Noetica #585.The timeout auditor exists because an unbounded 6h hang burned the Actions budget
this cycle. 122 unbounded jobs is the actionable red baseline.
Scoped for later waves (not built)
Control Teeth Register (keystone), billing/quota ingest, absent-verdict detector,
ephemeral-runner migration, import-graph test-reachability, recovery re-entrancy
harness — each with a proof obligation in the doc.
Safety / verification
git merge-treeclean against tools: four ways a check reported success without checking #1111 / ci: diagnostics-gate must fail closed on an unexplained skip [DO NOT MERGE until probe runs land] #1080 / DO NOT MERGE BEFORE #1080: fail diagnostics-gate on a job declared in the workflow but never wired into needs: #1082 (0 conflicts); no path overlap.validate-target-diagnostics.yml, a contested lane); wiring recommendation is in the doc.~/.claude/was modified.Do not merge yet — for review.
🤖 Generated with Claude Code