Skip to content

fix(http): refuse a repeated credential header with 400 (BACKLOG #2051) - #1817

Merged
wshallwshall merged 5 commits into
mainfrom
b179-2051-http-repeated-credential
Sep 29, 2026
Merged

wshallwshall merged 5 commits into
mainfrom
b179-2051-http-repeated-credential

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

Batch 179, item BACKLOG #2051. Own pull request because it changes a security control (intake authentication) and amends ADR 0154.

What it does

The Http() listener answers 400 duplicate credential header when a request repeats the credential header its active intake mode reads:

  • intake_api_key_header under api_key (default x-api-key, or a configured name);
  • Authorization under bearer.

Underscore spellings are folded, so x_api_key beside x-api-key counts as a repeat. Identical repeated values are refused too.

  • Where: in _authorize_head, after the rate-limiter consult and before any credential comparison or body read.
  • Charged and audited: the refusal counts against the failed-attempt budget and is audited as intake.auth_failed.
  • Nothing echoed: neither the header name nor its value appears in the response.
  • Bearer: the response also carries WWW-Authenticate: Bearer error="invalid_request" (RFC 6750 section 3.1).
  • Out of scope: a repeated header that the active mode does not read is still accepted.

ADR 0154 AC-11 said every missing or wrong credential gets one identical 401. A dated amendment (2026-09-29, a Manager ruling in batch 179) records the 400 and why. The header-name oracle is accepted because docs/CONNECTIONS.md states the name is not a secret.

  • Source branch: b179-2051-http-repeated-credential, head 93b32b0c38cff2d4d25b269971d52b138c1a617a
  • Commits: 5172fc7d86 (fix and tests), 59424e46dc (review round 1), a28528c0e0 (review round 2), 93b32b0c38 (ADR amendment)
  • Files: messagefoundry/transports/http_listener.py, tests/test_inbound_http_intake_auth.py, docs/SECURITY.md, docs/CONNECTIONS.md, docs/adr/0154-...md, docs/adr/README.md, CHANGELOG.md (BREAKING entry under [Unreleased] / Changed)

Builder report

Red/green: test_a_repeated_credential_header_is_refused_before_any_comparison ran its first 7 rows against unmodified origin/main code, and all 7 failed.

  • 6 rows got 202: wrong-then-right key, identical keys, a mixed-case name, a custom name, bearer wrong-then-right, and bearer identical.
  • The right-then-wrong row got 401.
  • All pass after the fix.

The underscore rows were added in review and were not run red.

Also added:

  • a spent-peer 429 test;
  • a health-probe test;
  • a test that a repeated header the mode does not read is still accepted. As a scope test, it passes on origin/main.

Checks run:

  • ruff check and ruff format --check: clean.
  • mypy messagefoundry: 302 files, clean.
  • mypy --explicit-package-bases tests: 1020 files, clean.
  • pytest over the 34 test files that match http_listener, HttpSource, intake_api_key or Http(, the doc-citing tests, and the tooling partition test: 4389 passed, 102 skipped, 1 xfailed.
  • ADR pass: 896 passed, 1 skipped, over the ADR, link and doc-drift tests.
  • The pre-commit hooks passed on each commit.

Checks skipped: the full suite and /simplify. There are no hosted-only legs.

Known defects: round-two review findings not repaired (two-round cap)

  1. The 400 is still a header-name oracle. When intake_auth_rate_limit is on, the failed-attempt budget caps it. When that setting is off, nothing caps it.
  2. The audit detail string is the same as for a wrong key. Only the log line and the connection event carry the reason.
  3. Altitude: a single(name) accessor on HttpRequest would make any future security header safe by default.
  4. Follow-up to file (not verified by the Manager): messagefoundry/api/security.py reads Authorization through Starlette's headers.get, which takes the first copy. That is the mirror of this defect on the engine API, at about line 322 and on the WebSocket path at about line 991.

Proposed ledger banner (for the Lander, after merge)

BUILD COMPLETED, NOT MERGED -- Http() listener answers 400 to a repeated intake credential header (api_key header or Authorization, underscore spellings folded), before any comparison, charged and audited as a failed attempt. ADR 0154 amended 2026-09-29.

wshallwshall added 4 commits September 29, 2026 15:47
The Http() listener's head parse kept the last of two same-named
headers, so `x-api-key: wrong` then `x-api-key: KEY` was accepted
while a front end that authenticates the first copy checked a
different credential. The head now records which header names
repeated, and _authorize_head refuses a request that repeats the
header the active mode reads (intake_api_key_header under api_key,
Authorization under bearer) with 400 framing_error. It runs before
the rate limiter and any comparison, writes no audit row, charges no
budget, and names neither the header nor its value. Identical copies
are refused too. none and mtls_subject read no credential header and
are unchanged.

Proposed PR title: fix(http): refuse a repeated credential header with 400 (BACKLOG #2051)
Banner: BUILD COMPLETED, NOT MERGED -- Http() listener answers 400 to a repeated intake credential header (api_key header or Authorization), before any comparison.
…ores (BACKLOG #2051)

Review round 1. The 400 tells a peer the credential header name was
right, which the 401 does not, so it now runs after the rate limiter
and is charged and audited as intake_auth_failed rather than being a
free, unaudited framing_error. Header repeats are counted with `_`
folded into `-`, so x_api_key beside x-api-key is refused as a repeat.
HttpRequest.repeated is now required. The credential header name is
resolved once at construction. SECURITY.md Table B and the intake row
name the 400; CONNECTIONS.md says "Each refusal". Tests add the
underscore row, a spent-budget 429 row and the health-probe rows.

Proposed PR title: fix(http): refuse a repeated credential header with 400 (BACKLOG #2051)
Banner: BUILD COMPLETED, NOT MERGED -- Http() listener answers 400 to a repeated intake credential header (api_key header or Authorization, underscore spellings folded), before any comparison, charged and audited as a failed attempt.
…pies (BACKLOG #2051)

Review round 2. The credential header name and its `_`-folded form
are resolved once at construction, and both the read and the repeat
check use them, so a configured name holding `_` is covered (new test
row). The bearer 400 carries WWW-Authenticate: Bearer
error="invalid_request" (RFC 6750 section 3.1). The intake warning
log names the refusal reason, a fixed string, so a repeated header is
told apart from a wrong key. The head parse folds each name once.
SECURITY.md Table B holds the rule; CONNECTIONS.md and CHANGELOG point
at it. The health-probe test asserts the event, not only the status.

Proposed PR title: fix(http): refuse a repeated credential header with 400 (BACKLOG #2051)
Banner: BUILD COMPLETED, NOT MERGED -- Http() listener answers 400 to a repeated intake credential header (api_key header or Authorization, underscore spellings folded), before any comparison, charged and audited as a failed attempt.
…LOG #2051)

AC-11 said every missing or wrong credential gets one identical 401. The
listener now answers 400 to a request that repeats the active mode's
credential header, before any comparison. Add a dated 2026-09-29
amendment (Manager ruling, batch 179): header status bullet, an italic
note in place on AC-11 plus a link to the new test, and an amendment
section. It points at SECURITY.md Table B for the rule, gives the RFC
9110 / RFC 6750 reason for 400, and accepts the header-name oracle
because CONNECTIONS.md says the name is not a secret and the refusal is
charged to the failed-attempt budget. The ADR index row records it too.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

QA (Builder, BUILDER.md step 11, code-review skill at xhigh on origin/main...HEAD, two rounds): Round 1: 10 findings, 7 fixed (header-name oracle by charging/auditing the 400, underscore bypass, no audit row, SECURITY.md contradicting code, repeated as a required field, double mode dispatch, Either->Each). Round 2: 14 findings, 8 fixed (folding the configured name, single source for the credential header name, bearer challenge, reason in log, health-probe test asserting the event, fold once, fewer doc copies, sentence length). Open per the two-round cap: (1) the 400 is still a rate-limited oracle for the header name, uncapped when intake_auth_rate_limit is off; (2) ADR 0154 AC-11 not amended [since done in 93b32b0]; (3) a single() accessor on HttpRequest; (4) the engine API Authorization duplicate; (5) the audit detail string matches a wrong key.

@wshallwshall wshallwshall added the qa Builder QA record posted; not a merge gate label Sep 29, 2026
@wshallwshall
wshallwshall added this pull request to the merge queue Sep 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Sep 29, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Lander conflict resolution: pushed 8402a8d756, a merge of origin/main (f9f0dfc90c) into this branch. It is a fast-forward of 93b32b0c38.

The only conflicted file was CHANGELOG.md: two BREAKING entries at the same spot, this PR's #2051 entry and main's #1352 PKCS#12 entry from PR 1815. I kept both, main's first. The other files auto-merged.

Check: the added and removed lines of git diff origin/main HEAD match this PR's own three-dot diff line for line (7 files, +240/-11). Re-armed.

@wshallwshall
wshallwshall added this pull request to the merge queue Sep 29, 2026
Merged via the queue into main with commit 9ec1ddf Sep 29, 2026
42 of 44 checks passed
@wshallwshall
wshallwshall deleted the b179-2051-http-repeated-credential branch September 29, 2026 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

qa Builder QA record posted; not a merge gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant