Skip to content

canonical_json silently stringifies unserializable values via default=str: hash stability depends on the value's str() representation #363

Description

@usmanimamu17-create

Problem

canonical_json (app/services/formatters.py):

def canonical_json(value: Any) -> str:
    return json.dumps(value, sort_keys=True, separators=(",", ":"), ensure_ascii=False, default=str)

default=str converts any non-serializable object (datetime, enum, custom class) to its str() output. The function is the foundation of the audit chain (AuditLogService._compute_entry_hash hashes canonical_json(data)), so hash stability depends on str() being deterministic and consistent across processes.

Consequences:

  • Hash instability across representations: str(Decimal("1.0")) vs str(Decimal("1")) differ; datetime str() output depends on the object's tz representation; a custom class's __str__ can change between versions — any of these silently changes the audit hash and makes chain verification (when fixed) report false corruption.
  • Data-loss masking: a value that fails JSON serialization (e.g. set, bytes) is recorded as its Python str()"{'a', 'b'}" — which is not the original data and may not even be stable JSON, breaking round-trip guarantees for audit details.
  • No warning is emitted: default=str is silent, so callers never learn that a value was stringified.

Root cause

default=str was added as a convenience for datetime/enum values without restricting what it applies to; every caller inherits it.

Why this is architecturally hard

  1. The audit chain hashes this function's output; changing it changes hashes of existing rows, so any change must either be backward-compatible for the serializers actually used (datetime via .isoformat(), enums via .value) or versioned with a chain re-verification story.
  2. Callers pass heterogeneous dicts (details with datetimes, SLAResult models, raw contract results); the correct approach is a shared serialization policy (e.g. a custom default that handles datetime/enum/Decimal explicitly and raises on anything else) used by every hash-relevant path.
  3. A regression test must feed values that str() renders non-deterministically (or types that should never serialize) and assert the output either fails loudly or is stable by construction.

Proposed design

Replace default=str with an explicit serializer handling datetime, enum.Enum, and Decimal canonically, raising TypeError for unknown types; add tests asserting stable output for the types the audit chain actually receives and a loud failure for opaque objects.

Acceptance criteria

Service

  • Hash-relevant serialization is stable for datetime/enum/Decimal inputs.
  • Opaque values fail loudly instead of being stringified.

Tests

  • Stability tests cover the types used in audit details.
  • Existing audit/parity tests pass.

Out of scope

The audit verifier serializer mismatch (tracked separately) and scrubber recursion.

Getting started

pytest tests/test_canonical_json.py tests/test_contract_parity.py -q
make typecheck

Good first files to read: app/services/formatters.py, app/services/audit_log.py, tests/test_canonical_json.py.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardThird CampaignCampaign: Third Campaignarea/auditImported campaign issue labelarea/dataImported campaign issue labelpriority/mediumStandard backlog item

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions