Skip to content

H2B: make canonical status bytes the evidence boundary - #47

Closed
Pigbibi wants to merge 2 commits into
mainfrom
feat/pert-feed-status-canonical-bytes-h2b
Closed

H2B: make canonical status bytes the evidence boundary#47
Pigbibi wants to merge 2 commits into
mainfrom
feat/pert-feed-status-canonical-bytes-h2b

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Scope

Fresh H2B from latest origin/main@294e30b8eed36f070299b1dc01d34955fb262918; PR #46 is permanently frozen evidence and was not reused.

This pure decision slice uses canonical JSON bytes as the sole persisted status evidence:

  • build_decision(validated_outcomes) returns status_bytes: bytes plus explicit ProducerDecision;
  • read_status(bytes) revalidates canonical encoding and semantic invariants, then returns fresh JSON-native built-in dict/list values;
  • caller mutation of read results cannot affect bytes, digest, or later reads;
  • digest is SHA-256 of status bytes, and readback equality is byte equality;
  • duplicate/noncanonical/tampered wire and malformed state/kind/rows/error/counter inputs fail closed with sanitized errors.

No MappingProxy/tuple wire payload, parser, I/O, workflow, publish, weekly, QAR, permission, or automation changes.

Validation

  • focused pytest: 8 passed
  • full pytest: 130 passed
  • compileall: passed
  • git diff --check: passed
  • ruff: unavailable in environment

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

🤖 Codex PR Review

Merge allowed: blocking findings were cleared by independent Codex arbitration

⚖️ Codex Review Arbitration

clear: Current source does not establish the contract asserted by this finding. src/political_event_tracking_research/rss_source_fetch.py does write source_items.csv in (published_at, item_id) order, but the repository’s published-artifact integrity contract is separately defined in src/political_event_tracking_research/artifacts.py and exemplified by data/live/source_manifest.json, which record whole-file sha256 for source_items.csv. In src/political_event_tracking_research/feed_status_canonical.py, row_digest and aggregate_row_digest are only exposed as canonical status fields; no test, schema, or documentation says they must preserve producer row order or be recomputable from the emitted CSV byte/order sequence. So the observation that _digest() canonicalizes row order does not prove a blocking contract violation against the current source of truth. This also does not conflict with the prior finding, which concerned read-time tamper validation of digest fields, not whether those digests are order-sensitive.

ℹ️ Other Findings

1. 🟡 [MEDIUM] Reliability in src/political_event_tracking_research/feed_status_canonical.py

read_status() decodes and json.loads() the entire byte blob without any size bound. A corrupted or malicious status file can therefore consume unbounded memory/CPU before the contract checks run, which is a realistic crash path for a workflow/parser that is supposed to fail closed on untrusted persisted evidence. (line 315)

Suggestion: Add a conservative MAX_STATUS_BYTES guard before UTF-8 decoding/JSON parsing, ideally derived from the maximum supported feed/field sizes, and reject oversized inputs with the same sanitized contract error path.


Review by Codex PR Review bot • PR

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c6542c51ba

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +284 to +285
if data["configured_feed_count"] != len(feeds) or data["feed_count"] != len(feeds):
_fail("status_counter_invalid")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject empty feed evidence on readback

When persisted bytes are read from the evidence boundary, a canonical status with feeds: [], all counters set to 0, and eligible_for_live_publication: true passes this check, even though build_decision() rejects the same condition with feed_config_empty. In a missing or misconfigured feed snapshot, read_status() can therefore return live-publication-eligible evidence that the producer would never emit.

Useful? React with 👍 / 👎.

if state in {"accepted", "quarantined"} and kind not in {"rss2", "atom"}:
_fail("feed_kind_invalid")
accepted_count = _integer(item["accepted_row_count"], "feed_counter_invalid")
_integer(item["rejected_row_count"], "feed_counter_invalid")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reconcile rejected row counters

This only type-checks each feed's rejected_row_count; the top-level rejected_row_count is never compared with the per-feed totals or with the producer's zero-rejection invariant. As a result, canonical status bytes can contain arbitrary rejected counters and still be accepted by read_status(), which can corrupt downstream quality gates or summaries that trust those counters.

Useful? React with 👍 / 👎.

Comment on lines +243 to +245
aggregate = _string(data["aggregate_row_digest"], "status_digest_invalid")
if not _DIGEST_RE.fullmatch(aggregate):
_fail("status_digest_invalid")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce the empty aggregate digest

When accepted_row_count is 0 (all feeds are failed/quarantined), the aggregate row set is known to be empty and build_decision() always emits _digest([]), but readback only checks that aggregate_row_digest is hex. A canonical status for an all-failed or all-quarantined run can therefore change this digest to any 64 hex characters and still pass read_status(), which breaks consumers using the aggregate digest as the row-set evidence.

Useful? React with 👍 / 👎.

Comment on lines +276 to +278
digest = _string(item["row_digest"], "status_digest_invalid")
if not _DIGEST_RE.fullmatch(digest):
_fail("status_digest_invalid")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce empty row digests for non-accepted feeds

For failed or quarantined feeds there are no rows by invariant, and build_decision() always emits the digest of an empty row list for those feed entries. read_status() only verifies that row_digest is shaped like hex, so canonical bytes for a failed/quarantined feed can carry any digest and still be accepted, breaking per-feed digest evidence for exactly the feeds whose rows are known to be empty.

Useful? React with 👍 / 👎.

Co-Authored-By: Codex <noreply@openai.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ef5b22da3e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +284 to +285
if state != "accepted" and digest != _EMPTY_DIGEST:
_fail("empty_digest_invalid")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject empty feed digests for accepted rows

When a feed is accepted with accepted_row_count > 0, read_status() still accepts row_digest equal to the known empty-row digest because this empty-digest check only applies to non-accepted feeds. build_decision() cannot emit the digest of [] for a nonempty accepted row list, so tampered canonical bytes can claim accepted rows while preserving empty feed evidence for consumers that trust the per-feed digest.

Useful? React with 👍 / 👎.

Comment on lines +307 to +308
if accepted_rows == 0 and data["aggregate_row_digest"] != _EMPTY_DIGEST:
_fail("empty_digest_invalid")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject empty aggregate digest with accepted rows

When accepted_rows > 0, there is no inverse check to reject aggregate_row_digest == _EMPTY_DIGEST; a canonical payload with accepted_row_count set to 1 and the aggregate digest set to sha256(b"[]") passes read_status(). build_decision() would not emit that digest for a nonempty accepted row set, so this lets persisted status evidence contradict its own row counters.

Useful? React with 👍 / 👎.

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