Skip to content

Wave B Auditor — proof bundle integrity report - #7

Merged
Ghenghis merged 1 commit into
developfrom
feat/wave-b-auditor
Apr 30, 2026
Merged

Ghenghis merged 1 commit into
developfrom
feat/wave-b-auditor

Conversation

@Ghenghis

Copy link
Copy Markdown
Owner

Summary

Independent third-party audit of the proof-bundle infrastructure end-to-end. Verdict: GREEN.

  • Built a fresh bundle with HERMES3D_PROOF_KEY=auditor-key bash scripts/build-bundle.sh: 05_truth_proof/bundles/ea909e8c5ae4-20260430T125728Z.zip (sha256 b25a5faa37e6…, 5533 bytes).
  • Wrote 04_testing/wave-b-reports/auditor_verify.py — an independent verifier that does NOT call conformance_runner.py. Reimplements the canonical-JSON HMAC-SHA256, file-hash walk, ledger cross-ref scan, forbidden-extras diff, and git cat-file -e commit binding from scratch.
  • Five positive checks (a–e) all PASS.
  • Three negative tests in temp dirs all PASS (i.e. the tampered bundle was rejected with the expected, specific error):
    • manifest tampered + sig replaced with forged HMAC -> signature mismatch
    • manifest-listed file removed -> file integrity: 1 missing
    • smuggled extra file added -> forbidden extras: ['smuggled.txt']
  • Cross-checked against conformance_runner.py --bundle: agrees.

Recommendations (non-blocking)

  • Port the "forbidden extras" check (d) into conformance_runner.verify_bundle. Currently it does not reject zip entries absent from manifest.files; smuggled files would slip past --bundle mode. The auditor catches them.
  • Re-run audit against a richer post-acceptance bundle to exercise the | proof_envelope rows in evidence_ledger.md.

Artifacts

  • 04_testing/wave-b-reports/auditor-20260430T125917Z.md — report
  • 04_testing/wave-b-reports/auditor_verify.py — independent verifier
  • 04_testing/wave-b-reports/negative_tests.py — adversarial harness

No proof-bundle code was modified. Tampered bundles were created in %TEMP% and never committed or pushed.

Test plan

  • Positive: independent verifier all PASS
  • Positive: conformance_runner.py --bundle agrees
  • Negative: tamper + wrong key resign rejected
  • Negative: removed manifest-listed file rejected
  • Negative: smuggled extra file rejected

- Adds 04_testing/wave-b-reports/auditor_verify.py: independent verifier
  that reimplements signature, file-hash, cross-ref, forbidden-extras,
  and commit-binding checks without calling conformance_runner.py.
- Adds negative_tests.py exercising three tampering scenarios in temp
  dirs (manifest tamper + wrong-key resign, missing manifest-listed
  file, smuggled extra file). All three tampered bundles are rejected.
- Adds auditor-20260430T125917Z.md report with positive checks (a–e)
  PASS, three negatives PASS, and recommendations.

Audited bundle: 05_truth_proof/bundles/ea909e8c5ae4-20260430T125728Z.zip
(sha256 b25a5faa37e6…, 5533 bytes, run_id ea909e8-20260430T125728Z).
@coderabbitai

coderabbitai Bot commented Apr 30, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@Ghenghis has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 38 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c015c2e2-9f83-4c61-9c6a-10afa414e6bd

📥 Commits

Reviewing files that changed from the base of the PR and between ea909e8 and ca44cca.

📒 Files selected for processing (3)
  • 04_testing/wave-b-reports/auditor-20260430T125917Z.md
  • 04_testing/wave-b-reports/auditor_verify.py
  • 04_testing/wave-b-reports/negative_tests.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/wave-b-auditor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 18 minutes and 38 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the Wave B Auditor report and its associated verification infrastructure, including an independent auditor script and a negative testing harness. The review feedback highlights several robustness improvements: handling missing manifest files in the auditor script to prevent crashes, adding command-line argument validation to the testing script, and utilizing a context manager for temporary directory management to ensure proper cleanup during failures.

Comment on lines +52 to +53
manifest = json.loads(zf.read("manifest.json").decode("utf-8"))
sig = json.loads(zf.read("manifest.sig").decode("utf-8"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The script will crash with a KeyError if manifest.json or manifest.sig are missing from the zip file. It's safer to check for their existence in names before attempting to read them, allowing the function to return a structured error report instead of an unhandled exception.

Suggested change
manifest = json.loads(zf.read("manifest.json").decode("utf-8"))
sig = json.loads(zf.read("manifest.sig").decode("utf-8"))
if "manifest.json" not in names or "manifest.sig" not in names:
if "manifest.json" not in names: result["errors"].append("manifest.json missing")
if "manifest.sig" not in names: result["errors"].append("manifest.sig missing")
result["ok"] = False
return result
manifest = json.loads(zf.read("manifest.json").decode("utf-8"))
sig = json.loads(zf.read("manifest.sig").decode("utf-8"))

Comment on lines +59 to +60
src = Path(sys.argv[1])
key = sys.argv[2]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The script lacks validation for command-line arguments. Running it without the required arguments will result in an IndexError. Adding a simple check with a usage message improves robustness.

Suggested change
src = Path(sys.argv[1])
key = sys.argv[2]
if len(sys.argv) < 3:
print(f"Usage: {sys.argv[0]} <bundle_zip> <key>", file=sys.stderr)
return 2
src = Path(sys.argv[1])
key = sys.argv[2]

def main() -> int:
src = Path(sys.argv[1])
key = sys.argv[2]
tmp = Path(tempfile.mkdtemp(prefix="wave-b-audit-neg-"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using tempfile.mkdtemp with manual cleanup is prone to leaving temporary directories behind if the script crashes or is interrupted. Using tempfile.TemporaryDirectory as a context manager ensures the directory is cleaned up automatically regardless of how the script exits.

@Ghenghis
Ghenghis merged commit 5844d33 into develop Apr 30, 2026
10 of 11 checks passed
@Ghenghis
Ghenghis deleted the feat/wave-b-auditor branch April 30, 2026 14:28
Ghenghis added a commit that referenced this pull request May 9, 2026
…/ PR #135) (#139)

Closes Bonus 12 finding #5 from PR #135 / bonus12-bug-finder.md.

Pre-fix
- _remote_release_tags swallowed every Exception and returned [].
  DNS poison, MITM TLS, GitHub 5xx, and rate-limit bans all collapsed
  silently into status="already_current" downstream, masking a stale
  Hermes Agent checkout (CWE-918 SSRF / OWASP CICD-SEC-1 fail-open).
- _latest_release had the same broad-catch shape on a parallel path.

Post-fix
- _remote_release_tags tightens except to the known network/parse modes:
  urllib.error.URLError (covers HTTPError 4xx/5xx, ContentTooShortError),
  socket.timeout, TimeoutError, ConnectionError, json.JSONDecodeError.
  Real failures raise HTTPException(502) with a _redact()-cleaned detail.
- non-list payload (200 + {object}) raises 502 (upstream contract violation).
- _latest_release distinguishes 404 (no /releases/latest yet -> soft warning)
  from real outages (5xx -> 502); URLError on latest endpoint preserves the
  soft-warning fast path because _remote_release_tags is the authoritative
  gate (if we got that far, tags came back successfully).

Behavior change
- update_status / staged_update will now flip from
  "outdated=False"/"already_current" to HTTP 502 during GitHub blips.
  This is the correct fail-closed posture for a supply-chain update channel
  (CWE-918 SSRF integrity face). Reviewers should confirm UI tolerates 502.

Tests added (12, all green)
- 04_testing/pytest/unit/test_agent_updates_ssrf.py
  * happy paths (success + genuinely empty release list)
  * socket.timeout, URLError, HTTP 5xx, malformed JSON, non-list payload
    all raise HTTPException(502)
  * Bearer-shaped strings in error detail are redacted
  * latest_release: 404 keeps soft warning; 5xx raises 502; URLError on
    latest keeps soft warning (downstream gate already authoritative)

Verification
- py_compile: OK
- Focused tests: 12/12 pass
- All agent_updates-keyed unit tests: 37/37 pass
- Pre-push hook: passed

Scope
- Bonus 12 finding #5 only - controlled-batch pattern continues.
- v0.13.0 update remains formally deferred.
- RC v2 commits 2-5 remain paused per user instruction.

Swarm provenance
- Agent 6 (general-purpose) of the 20-agent Blocker Elimination Swarm
  produced the unified diff this PR applies. Verified locally before
  apply; tests added by orchestrator.

Follow-ups (separate PRs)
- Bonus 12 #6 (config payload redaction asymmetry) - Agent 7 diff ready
- Bonus 12 #7 (apply_patch_proposal TOCTOU) - Agent 8 diff ready
- Bonus 12 #8 (_call_mcp_tool deadlock pattern) - Agent 9 diff ready
- Bonus 13 audit doc errata - Agent 10 diff ready
- GITHUB_TOKEN support to escape the 60-req/hr unauth rate limit
  (out-of-scope; new dominant cause of 502s post-fix)

References
- https://docs.python.org/3/library/urllib.error.html
- https://cwe.mitre.org/data/definitions/918.html (SSRF)
- https://owasp.org/www-project-top-10-ci-cd-security-risks/

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ghenghis added a commit that referenced this pull request May 9, 2026
…Bonus 12 #7 / PR #135) (#141)

Closes Bonus 12 finding #7 from PR #135 / bonus12-bug-finder.md.

Pre-fix order
- write tmp -> os.replace -> read target -> hash -> rollback (post-replace)
- No fsync on tmp fd; Windows write-back caching could lose bytes on
  power loss (etcd #13839 documents the same pattern producing zero-byte
  files on POSIX).
- Hash verified AFTER os.replace, so a corrupt write went live first.
- proof_events insert ran AFTER os.replace; SIGKILL between left the
  mutated file with no audit row to reconcile against.
- No fsync of the parent directory on POSIX, so the rename itself was
  not durable.

Post-fix order
1. Hash the proposed bytes BEFORE any FS mutation; reject up-front on
   mismatch with proposal manifest.
2. Open tmp via ``open(..., "wb")`` (binary, cross-platform), write,
   flush, ``os.fsync(fileno)``.
3. Record ``code_patch.replace_inflight`` proof event with proposal_id +
   pre_apply_snapshot_id + tmp_path BEFORE os.replace, so a recovery
   worker can reconcile a crash mid-replace.
4. ``os.replace`` (atomic on Windows + POSIX).
5. POSIX-only: fsync(parent_dir_fd) so the rename itself is durable;
   gracefully skipped on Windows (NTFS journals the rename and
   ``os.fsync`` on a Windows directory handle raises).
6. Defensive post-read hash check; any drift now indicates FS
   corruption, not a proposal mismatch.

Cross-platform note
- Initial draft used ``os.open(..., O_WRONLY|O_CREAT|O_TRUNC)`` for the
  tmp file, but on Windows that does NOT include ``O_BINARY`` by default
  and can translate ``\n`` -> ``\r\n``, breaking the post-replace hash.
  Tests caught this regression; fix uses ``open(..., "wb")`` which is
  binary on every platform.

Tests added (6, 5 pass + 1 POSIX-skip on Windows)
- 04_testing/pytest/unit/test_apply_patch_toctou.py
  * clean apply records inflight intent BEFORE replace, applied AFTER
  * hash mismatch blocks BEFORE replace; target file unchanged; NO
    inflight row written when manifest is rejected up-front
  * os.fsync called at least once on the tmp fd
  * simulated crash via os.replace OSError leaves target unchanged AND
    inflight marker in DB so recovery can reconcile
  * POSIX-only: parent-dir fsync makes rename durable (>=2 fsync calls)
  * Windows-only: function does not crash trying to fsync a directory

Verification
- py_compile: OK
- Focused tests: 5/5 pass + 1 skipped (POSIX-only on Windows host)
- code_operator + apply_patch + recovery_ledger: 67/68 pass (1 skipped)
- Pre-push hook: passed

Scope
- Bonus 12 finding #7 only.
- v0.13.0 update remains formally deferred.
- RC v2 commits 2-5 remain paused.

Swarm provenance
- Agent 8 of the 20-agent Blocker Elimination Swarm produced the diff.

References
- https://docs.python.org/3/library/os.html#os.fsync
- https://docs.python.org/3/library/os.html#os.replace
- etcd-io/etcd#13839
- https://lwn.net/Articles/457667/

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ghenghis added a commit that referenced this pull request May 9, 2026
…user) (#142)

User-mandated FIRST step in the E2E Blocker Elimination Program. Captures
every known blocker preventing Hermes3D OS from working end-to-end, with
columns: id / subsystem / severity / symptom / repro / squad / files /
receipts / fix PR / tests / rollback / status.

Initial state
- 20 blockers tracked (BLK-001..BLK-020)
- 8 verified (PRs #136-#141 closed Bonus 12 #1-#7 + the staged-update
  gate fake-pass surface from PR #136)
- 1 fixing (BLK-009 = Bonus 12 #8 MCP deadlock; Agent 9 diff ready)
- 1 fixing-doc (BLK-010 = Bonus 13 docs errata; Agent 10 diff ready)
- 1 upstream-blocked (BLK-011 = v0.13.0 upstream main continuously red)
- 1 paused (BLK-012 = RC v2 active repair loop)
- 7 open (BLK-013..BLK-020)

Squad map for 8 hardest blockers (A-H) included; each squad uses up to
6 agents (research / reproducer / fix-builder / test-builder /
sec-reviewer / integrator) with MCP locks.

Discovery audit pattern table included for the 12 classes the user
called out: TODO/FIXME, mock/fake, NotImplemented, suspicious pass,
xfail/skip, broad except, subprocess/Popen, urlopen/requests, zip/tar
extraction, secrets/env, disabled tests, hidden demo states.

E2E Definition of Done (14 items) checklist included.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ghenghis added a commit that referenced this pull request May 9, 2026
…026-05-09) (#147)

Closes the last two open Bonus 12 findings from PR #135 / bonus12-bug-finder.md.
Wave Agent 6 + synthesis at PR #146 confirmed both as ready-to-PR mechanical
edits.

#9 (P1) — _registry_path frozen-build IndexError
- Pre-fix: Path(__file__).resolve().parents[5] evaluated unguarded.
  On a frozen build / zipapp / nuitka, __file__ can be much shallower
  than 5 dirs from any plausible repo root, raising IndexError BEFORE
  the FileNotFoundError fallback to _registry_from_committed_proof()
  could trigger.
- Post-fix: each candidate path expression wrapped in its own try/except;
  malformed candidates are silently skipped so the documented
  FileNotFoundError fallback fires.

#10 (P0) — load_modules connection rollback
- Pre-fix: bare conn = connect(); ...; conn.commit(); conn.close() with
  no try/finally. A KeyError or sqlite3.IntegrityError mid-loop raised
  out of the loop with the connection still open, leaking the FD and
  WAL files on Windows. Half-loaded modules table left in DB.
- Post-fix:
  * with closing(connect()) as conn: always closes the connection.
  * try/except runs conn.rollback() on any exception before re-raising.
  * Half-committed state never persists.

Tests added (5, all green)
- 04_testing/pytest/unit/test_load_modules_resilience.py
  * #9: registry_path falls back when parents[5] raises IndexError
  * #9: registry_path returns first existing candidate (smoke)
  * #10: rollback runs exactly once on partial-load failure;
    commit() does NOT run; conn.closed is True
  * #10: clean path commits once and closes
  * #10: source-level pin — with-closing(connect()) pattern + conn.rollback()
    must remain in source (catches accidental revert)

Verification
- py_compile: OK
- Focused tests: 5/5 pass
- Pre-push hook: passed

Scope
- Bonus 12 batch fully closed (10/10): #1-#7 done; #8 partial
  (BLK-009 escalated upstream); #9 + #10 in this PR.

Swarm provenance
- Wave Agent 6 of the 10-agent Remaining/Skipped Wave produced the
  diff sketches; orchestrator implemented + tested.

References
- https://docs.python.org/3/library/contextlib.html#contextlib.closing
- https://www.sqlite.org/wal.html (WAL file FD-leak class)
- https://owasp.org/www-project-top-10-ci-cd-security-risks/

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant