feat(spec): signed waiver records for the #947 assessment chain (#947 increment 6) - #1060
Conversation
…increment 6)
app.spec.waiver_record — a pure, IO-free function pair that makes a
normalization-assessment waiver tamper-evident:
- sign_waiver(waiver, *, signer, signed_at, key_id, key) deep-copies the
waiver and returns {"waiver", "signature"} where signature.value is an
HMAC-SHA256 over the canonical JSON of the waiver with the signer /
signed_at / key_id metadata folded in as _meta, so altering the
metadata invalidates the signature exactly as altering the body does.
- verify_waiver_signature(record, *, key) recomputes and compares with
hmac.compare_digest. Body/metadata tamper or wrong key -> False;
missing signature or non-hmac-sha256 algo -> ValueError.
Canonical form sorts keys at every level (key-order independent). The
secret key is caller-supplied and never stored, logged, or echoed back.
20 test cases (round-trip, per-field tamper, wrong key, JSON round-trip,
determinism, blank-metadata rejection). mypy clean, interrogate 100%.
Cites NIST FIPS 198-1 and RFC 8785; doctoring note updated.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013SeQS8tSee5QVeyGpJ9SaY
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| References (APA 7th): | ||
|
|
||
| * National Institute of Standards and Technology. (2008). *The keyed-hash | ||
| message authentication code (HMAC)* (FIPS PUB 198-1). | ||
| https://doi.org/10.6028/NIST.FIPS.198-1 | ||
| * Rundgren, A., Jordan, B., & Erdtman, S. (2020). *JSON Canonicalization | ||
| Scheme (JCS)* (RFC 8785). RFC Editor. | ||
| https://doi.org/10.17487/RFC8785 |
There was a problem hiding this comment.
🔍 Research grounding is incomplete
The signing feature adds standards citations but no academic paper PDF or redistribution assessment. Repository governance requires this grounding for substantive features.
Was this helpful? React with 👍 or 👎 to provide feedback.
| """Compute the HMAC-SHA256 hex digest over the waiver plus its signature meta.""" | ||
|
|
||
| return hmac.new( | ||
| key, _canonical({**waiver, "_meta": meta}), hashlib.sha256 |
There was a problem hiding this comment.
🟥 Reserved metadata bypasses waiver integrity
When a waiver contains _meta, sign_waiver excludes its value from the signature. Attackers can alter that field while verification still succeeds.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return json.dumps( | ||
| waiver, sort_keys=True, separators=(",", ":"), default=str | ||
| ).encode("utf-8") |
There was a problem hiding this comment.
🟨 Ambiguous values share valid signatures
default=str gives distinct waiver values identical signed bytes. A numeric object can become an authenticated string without invalidating verification.
Was this helpful? React with 👍 or 👎 to provide feedback.
| meta = {field: str(signature.get(field, "")) for field in _META_FIELDS} | ||
| expected = _expected_value(waiver, meta, bytes(key)) | ||
| return hmac.compare_digest(expected, str(signature.get("value", ""))) |
There was a problem hiding this comment.
🟨 Metadata type changes evade verification
verify_waiver_signature coerces metadata with str. Attackers can replace a signed string with an equal-looking non-string while verification still succeeds.
Was this helpful? React with 👍 or 👎 to provide feedback.
seonghobae
added
area: api
ChatGPT Codex Connector
Gate still frozen (main@8dc74692; .github#1531 queue oscillating ~1700-1900, no drainage). iter36 gap-baseline consolidation: record PR #1060 (pure sign_waiver / verify_waiver_signature HMAC-SHA256 tamper-evidence pair) in the #947 "This loop's increment PRs" list. Stacked-PR count 17 -> 18; #947 merge-wave chain extended to #1048 -> #1060; #947 "Remaining increments" reworded (signing core landed, persistence remains). CHANGELOG [Docs] bullet count 17 -> 18. MD018-clean. Docs-only. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013SeQS8tSee5QVeyGpJ9SaY
1 participant
What
Sixth #947 increment. Adds
app.spec.waiver_record— a pure, IO-freefunction pair that makes a normalization-assessment waiver tamper-evident
so an auditor can trust one without re-reviewing it.
sign_waiver(waiver, *, signer, signed_at, key_id, key)deep-copies thewaiver and returns
{"waiver", "signature": {"algo", "signer", "signed_at", "key_id", "value"}}.valueis an HMAC-SHA256 over the canonical JSON ofthe waiver with the signature metadata folded in as
_meta, sochanging the signer or the timestamp invalidates the signature exactly as
changing the waiver body does.
verify_waiver_signature(record, *, key)recomputes that HMAC and compareswith
hmac.compare_digest(constant time). Body/metadata tamper or a wrongkey →
False; a missing signature or a non-hmac-sha256algo→ValueError.different key order still verifies. The secret
keyis caller-supplied andnever stored, logged, or echoed into the record.
Why
The assessment modules already accept caller-supplied waivers but trust them
as-is. This is the signing/verification core for an audit trail; persisting
the signed records stays deferred (storage-layer work, tracked in the
doctoring note).
Tests / checks
tamper, per-field signature-metadata tamper, doctored digest, wrong key,
missing signature, unsupported algo, key-order independence, JSON
round-trip, determinism, blank-metadata / empty-key rejection.
PYTHONPATH=. mypy appclean (75 files);interrogate100%;tests/test_docstrings.pygreen.Standards (APA 7th)
Stack
Branched off
feat/transitive-dependency-assessment-20260902(PR #1048, the#947 chain tip). Blocked from merge by the org CI incident
ContextualWisdomLab/.github#1531like the rest of the loop's stack.🤖 Generated with Claude Code