-
Notifications
You must be signed in to change notification settings - Fork 0
feat(spec): signed waiver records for the #947 assessment chain (#947 increment 6) #1060
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
seonghobae
wants to merge
1
commit into
feat/transitive-dependency-assessment-20260902
Choose a base branch
from
feat/waiver-signature-20260902
base: feat/transitive-dependency-assessment-20260902
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+374
−1
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| """Tamper-evident signing for normalization-assessment waiver records. | ||
|
|
||
| The assessment modules in this package (:mod:`app.spec.normalization_assessment` | ||
| and :mod:`app.spec.transitive_dependency_assessment`) accept caller-supplied | ||
| *waivers*: small records that say "this finding is a deliberate, reviewed | ||
| exception, not a defect". Today those waivers are trusted as-is. For an audit | ||
| trail an enterprise buyer can rely on, a waiver needs to be **tamper-evident**: | ||
| a reviewer signs it once, and anyone can later check that neither the waiver | ||
| body nor the "who signed it / when / with which key" metadata was altered | ||
| afterwards. | ||
|
|
||
| This module does exactly that and nothing more: | ||
|
|
||
| * :func:`sign_waiver` takes a waiver ``dict`` plus the signer identity, an | ||
| ISO-8601 timestamp, a key id, and the secret key bytes. It returns a new | ||
| record ``{"waiver": <deep copy>, "signature": {...}}`` whose ``signature`` | ||
| carries an HMAC-SHA256 over the canonical JSON of the waiver *with the | ||
| signature metadata folded in*, so changing the signer or the timestamp | ||
| invalidates the signature just as changing the waiver body would. | ||
| * :func:`verify_waiver_signature` recomputes that HMAC from ``record["waiver"]`` | ||
| and ``record["signature"]`` and compares it in constant time. | ||
|
|
||
| The secret key never leaves the caller: this module neither stores it, logs | ||
| it, nor puts it (or any plaintext derived from it) into the returned record. | ||
| It is a pure function pair with no database, network, or filesystem access. | ||
|
|
||
| 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 | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import hashlib | ||
| import hmac | ||
| import json | ||
| from copy import deepcopy | ||
| from typing import Any | ||
|
|
||
| WAIVER_SIGNATURE_ALGO = "hmac-sha256" | ||
| """Identifier stored in every signature; the only algorithm this module accepts.""" | ||
|
|
||
| _META_FIELDS = ("signer", "signed_at", "key_id") | ||
|
|
||
|
|
||
| def _canonical(waiver: dict[str, Any]) -> bytes: | ||
| """Return a deterministic byte string for ``waiver``. | ||
|
|
||
| Keys are sorted at every level and separators are tight, so two dicts that | ||
| are equal as Python objects produce identical bytes regardless of the order | ||
| their keys were inserted. ``default=str`` lets values such as ``datetime`` | ||
| or ``Decimal`` serialize instead of raising; the same Python value always | ||
| stringifies the same way, which is all a signature needs. | ||
| """ | ||
|
|
||
| return json.dumps( | ||
| waiver, sort_keys=True, separators=(",", ":"), default=str | ||
| ).encode("utf-8") | ||
|
Comment on lines
+61
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
|
|
||
| def _require_non_empty_str(value: object, field: str) -> str: | ||
| """Return ``value`` unchanged, or raise :class:`ValueError` naming ``field``.""" | ||
|
|
||
| if not isinstance(value, str) or not value: | ||
| raise ValueError(f"{field} must be a non-empty string") | ||
| return value | ||
|
|
||
|
|
||
| def _expected_value(waiver: dict[str, Any], meta: dict[str, str], key: bytes) -> str: | ||
| """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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ).hexdigest() | ||
|
|
||
|
|
||
| def sign_waiver( | ||
| waiver: dict[str, Any], | ||
| *, | ||
| signer: str, | ||
| signed_at: str, | ||
| key_id: str, | ||
| key: bytes, | ||
| ) -> dict[str, Any]: | ||
| """Return a signed, tamper-evident copy of ``waiver``. | ||
|
|
||
| Args: | ||
| waiver: The waiver body to sign. It is deep-copied into the result, so | ||
| the caller's dict is never mutated and later edits to it do not | ||
| affect the signed record. | ||
| signer: Who approved the waiver (a person or system identity). Required, | ||
| non-empty. | ||
| signed_at: When it was approved, as an ISO-8601 string. Required, | ||
| non-empty; this module records it verbatim and does not parse it. | ||
| key_id: Which signing key was used, so a verifier can pick the right | ||
| secret without trial and error. Required, non-empty. | ||
| key: The secret key bytes for the HMAC. Required, non-empty. Never | ||
| stored, logged, or echoed back in the result. | ||
|
|
||
| Returns: | ||
| ``{"waiver": <deep copy of waiver>, "signature": {"algo", "signer", | ||
| "signed_at", "key_id", "value"}}`` where ``value`` is the HMAC-SHA256 | ||
| hex digest binding the waiver body to the three metadata fields. | ||
|
|
||
| Raises: | ||
| ValueError: If ``signer``, ``signed_at``, or ``key_id`` is not a | ||
| non-empty string, or if ``key`` is empty / not ``bytes``. | ||
| """ | ||
|
|
||
| meta = { | ||
| "signer": _require_non_empty_str(signer, "signer"), | ||
| "signed_at": _require_non_empty_str(signed_at, "signed_at"), | ||
| "key_id": _require_non_empty_str(key_id, "key_id"), | ||
| } | ||
| if not isinstance(key, (bytes, bytearray)) or not key: | ||
| raise ValueError("key must be non-empty bytes") | ||
|
|
||
| return { | ||
| "waiver": deepcopy(waiver), | ||
| "signature": { | ||
| "algo": WAIVER_SIGNATURE_ALGO, | ||
| **meta, | ||
| "value": _expected_value(waiver, meta, bytes(key)), | ||
| }, | ||
| } | ||
|
|
||
|
|
||
| def verify_waiver_signature(record: dict[str, Any], *, key: bytes) -> bool: | ||
| """Return ``True`` iff ``record``'s signature matches its waiver body. | ||
|
|
||
| Recomputes the HMAC-SHA256 from ``record["waiver"]`` and the ``signer`` / | ||
| ``signed_at`` / ``key_id`` inside ``record["signature"]``, then compares it | ||
| to the stored ``value`` with :func:`hmac.compare_digest` (constant time). | ||
| Any change to the waiver body or to a signature metadata field makes this | ||
| return ``False``; a wrong ``key`` also returns ``False``. | ||
|
|
||
| Args: | ||
| record: A record produced by :func:`sign_waiver` (or one claiming to | ||
| be). Must have a ``waiver`` dict and a ``signature`` dict whose | ||
| ``algo`` is :data:`WAIVER_SIGNATURE_ALGO`. | ||
| key: The secret key bytes to verify against. | ||
|
|
||
| Raises: | ||
| ValueError: If ``record`` is missing ``waiver`` or ``signature``, if | ||
| either is not a dict, or if the signature's ``algo`` is not | ||
| :data:`WAIVER_SIGNATURE_ALGO`. | ||
| """ | ||
|
|
||
| if not isinstance(record, dict) or "signature" not in record: | ||
| raise ValueError("record must contain a 'signature'") | ||
| waiver = record.get("waiver") | ||
| signature = record["signature"] | ||
| if not isinstance(waiver, dict) or not isinstance(signature, dict): | ||
| raise ValueError("record 'waiver' and 'signature' must both be objects") | ||
| if signature.get("algo") != WAIVER_SIGNATURE_ALGO: | ||
| raise ValueError(f"unsupported signature algo: {signature.get('algo')!r}") | ||
|
|
||
| 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", ""))) | ||
|
Comment on lines
+163
to
+165
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,170 @@ | ||
| """Tests for :mod:`app.spec.waiver_record` — signed, tamper-evident waivers.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| from typing import Any | ||
|
|
||
| import pytest | ||
|
|
||
| from app.spec.waiver_record import ( | ||
| WAIVER_SIGNATURE_ALGO, | ||
| sign_waiver, | ||
| verify_waiver_signature, | ||
| ) | ||
|
|
||
| _KEY = b"unit-test-secret-key-0123456789ab" | ||
| _OTHER_KEY = b"a-different-secret-key-0123456789" | ||
|
|
||
|
|
||
| def _waiver() -> dict[str, Any]: | ||
| """Return a representative waiver body (matches the assessment-module shape).""" | ||
|
|
||
| return { | ||
| "scope": {"relation": "sales.invoice_line", "kind": "candidate_3nf_split"}, | ||
| "owner": "data-architecture-guild", | ||
| "reason": "denormalized on purpose for the reporting read model", | ||
| "review_date": "2026-09-01", | ||
| "expiry": "2027-03-01", | ||
| } | ||
|
|
||
|
|
||
| def _sign(waiver: dict[str, Any] | None = None) -> dict[str, Any]: | ||
| """Sign ``waiver`` (or the default) with fixed metadata for reuse in tests.""" | ||
|
|
||
| return sign_waiver( | ||
| waiver if waiver is not None else _waiver(), | ||
| signer="reviewer@example.test", | ||
| signed_at="2026-09-02T10:00:00Z", | ||
| key_id="waiver-key-2026-09", | ||
| key=_KEY, | ||
| ) | ||
|
|
||
|
|
||
| def test_round_trip_verifies_true() -> None: | ||
| """A freshly signed record verifies against the same key.""" | ||
|
|
||
| record = _sign() | ||
| assert record["signature"]["algo"] == WAIVER_SIGNATURE_ALGO | ||
| assert verify_waiver_signature(record, key=_KEY) is True | ||
|
|
||
|
|
||
| def test_signing_does_not_mutate_caller_waiver() -> None: | ||
| """The caller's dict is deep-copied, not referenced, by the signed record.""" | ||
|
|
||
| original = _waiver() | ||
| record = _sign(original) | ||
| original["reason"] = "changed after signing" | ||
| assert record["waiver"]["reason"] == "denormalized on purpose for the reporting read model" | ||
| assert verify_waiver_signature(record, key=_KEY) is True | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("field", ["owner", "reason", "review_date", "expiry"]) | ||
| def test_tampering_a_waiver_field_fails_verification(field: str) -> None: | ||
| """Editing any waiver body field after signing is detected.""" | ||
|
|
||
| record = _sign() | ||
| record["waiver"][field] = "tampered" | ||
| assert verify_waiver_signature(record, key=_KEY) is False | ||
|
|
||
|
|
||
| def test_tampering_nested_scope_fails_verification() -> None: | ||
| """Editing a nested waiver value is detected too.""" | ||
|
|
||
| record = _sign() | ||
| record["waiver"]["scope"]["relation"] = "sales.something_else" | ||
| assert verify_waiver_signature(record, key=_KEY) is False | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("field", ["signer", "signed_at", "key_id"]) | ||
| def test_tampering_signature_metadata_fails_verification(field: str) -> None: | ||
| """Changing who/when/which-key without re-signing is detected.""" | ||
|
|
||
| record = _sign() | ||
| record["signature"][field] = "tampered" | ||
| assert verify_waiver_signature(record, key=_KEY) is False | ||
|
|
||
|
|
||
| def test_tampering_signature_value_fails_verification() -> None: | ||
| """A doctored HMAC digest does not verify.""" | ||
|
|
||
| record = _sign() | ||
| record["signature"]["value"] = "0" * 64 | ||
| assert verify_waiver_signature(record, key=_KEY) is False | ||
|
|
||
|
|
||
| def test_wrong_key_fails_verification() -> None: | ||
| """Verification with a different secret key returns False, not an error.""" | ||
|
|
||
| record = _sign() | ||
| assert verify_waiver_signature(record, key=_OTHER_KEY) is False | ||
|
|
||
|
|
||
| def test_missing_signature_raises_value_error() -> None: | ||
| """A record without a signature is a programming error, not a False.""" | ||
|
|
||
| with pytest.raises(ValueError, match="signature"): | ||
| verify_waiver_signature({"waiver": _waiver()}, key=_KEY) | ||
|
|
||
|
|
||
| def test_unsupported_algo_raises_value_error() -> None: | ||
| """Only HMAC-SHA256 is accepted; anything else is rejected loudly.""" | ||
|
|
||
| record = _sign() | ||
| record["signature"]["algo"] = "hmac-sha1" | ||
| with pytest.raises(ValueError, match="algo"): | ||
| verify_waiver_signature(record, key=_KEY) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("bad", ["", None, 0]) | ||
| def test_blank_metadata_is_rejected_at_signing(bad: object) -> None: | ||
| """signer / signed_at / key_id must each be a non-empty string.""" | ||
|
|
||
| for field in ("signer", "signed_at", "key_id"): | ||
| kwargs: dict[str, Any] = { | ||
| "signer": "s", | ||
| "signed_at": "t", | ||
| "key_id": "k", | ||
| "key": _KEY, | ||
| } | ||
| kwargs[field] = bad | ||
| with pytest.raises(ValueError, match=field): | ||
| sign_waiver(_waiver(), **kwargs) | ||
|
|
||
|
|
||
| def test_empty_key_is_rejected_at_signing() -> None: | ||
| """An empty signing key is refused.""" | ||
|
|
||
| with pytest.raises(ValueError, match="key"): | ||
| sign_waiver( | ||
| _waiver(), | ||
| signer="s", | ||
| signed_at="t", | ||
| key_id="k", | ||
| key=b"", | ||
| ) | ||
|
|
||
|
|
||
| def test_canonical_form_is_key_order_independent() -> None: | ||
| """Two waivers equal as dicts but built in different key order verify alike.""" | ||
|
|
||
| a = {"alpha": 1, "beta": {"x": 1, "y": 2}} | ||
| b = {"beta": {"y": 2, "x": 1}, "alpha": 1} | ||
| record_a = _sign(a) | ||
| # Swap in the differently-ordered but equal body; signature must still hold. | ||
| record_a["waiver"] = b | ||
| assert verify_waiver_signature(record_a, key=_KEY) is True | ||
|
|
||
|
|
||
| def test_record_survives_json_round_trip() -> None: | ||
| """Serializing and reloading the record does not break verification.""" | ||
|
|
||
| record = _sign() | ||
| reloaded = json.loads(json.dumps(record)) | ||
| assert verify_waiver_signature(reloaded, key=_KEY) is True | ||
|
|
||
|
|
||
| def test_signing_is_deterministic() -> None: | ||
| """Signing the same inputs twice yields the same digest.""" | ||
|
|
||
| assert _sign()["signature"]["value"] == _sign()["signature"]["value"] |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 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.