Skip to content

fix: resolve the 3 behavior-change audit Lows#83

Merged
lesnik512 merged 1 commit into
mainfrom
fix/audit-low-behavior-changes
Jun 14, 2026
Merged

fix: resolve the 3 behavior-change audit Lows#83
lesnik512 merged 1 commit into
mainfrom
fix/audit-low-behavior-changes

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Summary

Completes the 2026-06-14 deep-audit Low cluster. The safe 7 findings shipped in #82; this PR handles the 3 behavior changes, each in its own reviewable diff because each changes observable behavior. Decisions were chosen up front (clamp / add probe / eager validation).

Changes

1. graceful_timeout=None no longer hangs the drain

anyio.move_on_after(None) has deadline inf, so a single wedged handler made stop() (hence pod shutdown) hang forever. The drain now clamps None to a finite fallback (_DEFAULT_DRAIN_TIMEOUT_SECONDS = 15.0, mirroring the broker default); None stays unbounded where FastStream uses it that way (ping()).

2. validate_schema() now probes the _lease_ck CHECK

Alembic's compare_metadata has no check-constraint comparator, so a missing/altered <table>_lease_ck (the half-set-lease guard) passed validation silently. Added _validate_check_constraints_sync, a direct pg_constraint probe mirroring the existing partial-index probe.

⚠️ New failure mode (intended): a DB lacking the CHECK that passed validate_schema() before will now fail it.

3. OutboxResponse validates eagerly

The activate-mutex + tz-aware checks now run in OutboxResponse.__init__, so a misconfigured response raises at the return OutboxResponse(...) site instead of deferring to dispatch — where it masqueraded as a handler failure and exhausted the inbound row's retry budget. OutboxPublishCommand still re-checks on as_publish_command() (authoritative source).

Docs

  • CLAUDE.md: validate_schema CHECK probe + graceful_timeout=None clamp.
  • architecture/drain.md: the None clamp rationale.
  • Audit record: all 3 marked RESOLVED (the validate_schema one upgraded from PARTIAL).

Tests (+5)

  • test_outbox_response_rejects_{naive_activate_at,both_activate_args}_eagerly (unit)
  • test_graceful_timeout_none_still_bounds_drain (integration; patches the fallback small so it's fast — would hang without the clamp)
  • test_validate_schema_fails_when_lease_check_constraint_{missing,predicate_wrong} (integration; happy path covered by the existing passes-for-correct-table test)

Verification

  • Full suite: 492 passed, 100% coverage held.
  • ruff format, ruff check, ty check all clean.

With this merged, all 12 confirmed audit findings are resolved.

🤖 Generated with Claude Code

Completes the 2026-06-14 deep-audit Low cluster (the safe 7 shipped in #82).
Each changes observable behavior, hence a separate reviewed PR.

- graceful_timeout=None no longer hangs drain: subscriber stop() clamps None to
  a finite fallback (_DEFAULT_DRAIN_TIMEOUT_SECONDS=15.0) so move_on_after always
  has a real deadline; None stays unbounded for ping(). One wedged handler can no
  longer make stop() (pod shutdown) hang forever.
- validate_schema() now probes pg_constraint for <table>_lease_ck via
  _validate_check_constraints_sync (alembic has no check-constraint comparator),
  flagging a missing or drifted lease-pairing CHECK. NOTE: new failure mode — a DB
  lacking the CHECK that passed before will now fail validation (intended).
- OutboxResponse.__init__ runs the activate-mutex + tz-aware checks eagerly, so a
  misconfigured response raises at the return site instead of masquerading as a
  handler failure and exhausting the inbound row's retry budget. OutboxPublishCommand
  still re-checks on as_publish_command() (authoritative source).

Docs: CLAUDE.md (validate_schema CHECK probe; graceful_timeout None clamp),
architecture/drain.md, audit record marked resolved.

Tests: +5 (2 OutboxResponse unit, graceful_timeout clamp, 2 lease_ck validate).
Full suite 492 passed, 100% coverage held; ruff + ty clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lesnik512
lesnik512 merged commit 28489a4 into main Jun 14, 2026
3 checks passed
@lesnik512
lesnik512 deleted the fix/audit-low-behavior-changes branch June 14, 2026 08:33
lesnik512 added a commit that referenced this pull request Jun 14, 2026
Third blind multi-agent pass (8 finders -> 6 adversarial verifiers).
Headline: 1 High (propagate_inbound_headers cross-content-type relay
poisons the inbound row, F5-01) that passes 1-2 missed, plus an
incomplete prior fix (#83 left queue/session validation deferred on
OutboxResponse, F4-01/02). Findings-doc only; no code changes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lesnik512 added a commit that referenced this pull request Jun 14, 2026
…ate queue/session (F4-01/02/06/10) (#86)

PR #83 made OutboxResponse validate activate_*/tz eagerly but left queue and
session deferred to as_publish_command() at dispatch — so a bad queue/session in
a returned OutboxResponse still masqueraded as a handler failure and burned the
inbound row's retry budget (the exact footgun #83 targeted). broker.publish_batch
likewise skipped queue validation on the empty-batch path.

Collapse all the per-site checks into one shared _validate_publish_args(context,
*, queue, session, activate_in, activate_at) in response.py (order: activate-args
-> session -> queue, so OutboxResponse's eager activate check still fires before
the session check). It's called by the OutboxPublishCommand constructor,
OutboxResponse.__init__, and broker.publish_batch's empty branch, so every real
publish entry point — including an empty batch — rejects the same
misconfigurations identically and eagerly. as_publish_command() re-runs it, so
OutboxPublishCommand stays the authoritative source and the eager checks can't
drift.

Three dual-fire-guard tests passed session=None to OutboxResponse as a shortcut;
corrected to a valid session (the latent misuse F4-02 surfaces). _validate_activate_args
remains for the intentionally-session-lenient test fakes.

Tests in tests/test_unit.py; CLAUDE.md + pass-3 findings doc updated.

Co-authored-by: Claude Opus 4.8 (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