[PetrefiedThunder]

An explicitly empty or whitespace API_KEY_HASH_PEPPER could fall back to an earlier nonempty value in cached Settings after #254. Production authentication configuration now fails closed when that environment variable is present but blank; an absent variable still uses Settings.

Two regression cases prime the actual Settings cache, blank the environment variable, and require APIKeyHashingUnavailable. Both failed before the fix. The complete scrypt test file passes: 36 passed. Configured pre-commit (including mypy, Ruff and secret checks), staged gitleaks, and committed-branch TruffleHog passed.

Independent Muse review: PASS for the exact two-file diff SHA256 29628d2bf4c4bada1735076041f5134c0823ff288185d5dd3bf51c32146b3988. Codex independently inspected the diff and reran all 36 tests. Muse performed a read-only inline-diff review and did not execute tests. No schema, public API, key format, or deployment change.

Head: 7814d3e1f2ef3adec43fb710029bc2c6b97ba504. CI must be green before merge.

---

Note

High Risk
Changes how the API-key hash pepper is resolved when operators blank the env var, directly affecting production authentication and startup guards.

Overview
Production API-key hashing no longer treats an explicitly set but empty API_KEY_HASH_PEPPER as “missing” and fall back to .env or cached Settings. _configured_pepper() now keys off API_KEY_HASH_PEPPER in os.environ: if the variable is present, only its value (after strip) counts; blank or whitespace yields no pepper and fail-closed startup/auth in production-like environments. If the variable is unset, behavior is unchanged—Settings still supply the pepper.

A parametrized regression test primes the real get_settings() cache with a valid env pepper, then sets the env to "" or whitespace and asserts validate_api_key_hashing_config() raises APIKeyHashingUnavailable.

^{Reviewed by Cursor Bugbot for commit 45dd80b. Bugbot is set up for automated code reviews on this repo. Configure here.}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
agent-middleware-web	Ready	Preview	Oct 30, 2026 8:16am UTC

[PetrefiedThunder]

An explicitly empty or whitespace API_KEY_HASH_PEPPER could fall back to an earlier nonempty value in cached Settings after #254. Production authentication configuration now fails closed when that environment variable is present but blank; an absent variable still uses Settings.

Two regression cases prime the actual Settings cache, blank the environment variable, and require APIKeyHashingUnavailable. Both failed before the fix. The complete scrypt test file passes: 36 passed. Configured pre-commit (including mypy, Ruff and secret checks), staged gitleaks, and committed-branch TruffleHog passed.

Independent Muse review: PASS for the exact two-file diff SHA256 29628d2bf4c4bada1735076041f5134c0823ff288185d5dd3bf51c32146b3988. Codex independently inspected the diff and reran all 36 tests. Muse performed a read-only inline-diff review and did not execute tests. No schema, public API, key format, or deployment change.

Head: 7814d3e1f2ef3adec43fb710029bc2c6b97ba504. CI must be green before merge.

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: f97a1771-2ef5-4817-acb7-1b06369dcbad)

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-26T15:53:06.379639Z	7814d3e	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: agentmiddleware/agent-middleware
• Branch: codex/fail-closed-cached-pepper-1026
• File: .coderabbit.yaml
• Commit: 45dd80b
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 91af24fb-e2ff-4783-a0fb-30d0cc52214b

Commits

Reviewing files that changed from the base of the PR and between 7814d3e and 45dd80b.

Files selected for processing: 1

• gateway/app/services/api_key_service.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Walkthrough

_configured_pepper now uses API_KEY_HASH_PEPPER when the environment variable is present, including when its value is empty. It strips the selected value before the existing return logic. A parameterized regression test checks that empty and whitespace-only environment values raise APIKeyHashingUnavailable after settings were cached with a valid pepper.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: Minimal · up to 45dd8

The intended fail-closed pepper behavior is implemented and covered by regression tests; proceed with normal CI checks.

Security architecture risk: Minimal · up to 45dd8

The change strengthens fail-closed behavior: an explicitly blank pepper no longer revives a previously configured secret. No new externally controlled input, privilege, or authentication bypass was identified in the changed behavior.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The affected authority is the process-wide authentication pepper, shared by API-key operations and agent-comms digests in consumers using this configuration. The observed exposure is configuration-dependent failure of those consumers, not a newly introduced tenant-selectable secret or request-controlled configuration source.

Security Findings and Attack Paths

• inferred — The routed public-entrypoint signal resolves to an internal configuration helper, not a new network entrypoint. Its changed branch removes stale-secret fallback rather than granting authority. No introduced authentication bypass was identified through the inspected consumers.

Trust Boundaries and Controls

• observed — The resolver accepts process environment or Settings values, not presented API-key data. The existing environment-classification control still restricts the public development pepper to non-production-like environments. API-key validation obtains a usable pepper before proceeding to verification, so the inspected verification path does not bypass blank-pepper rejection through its cache.

Pre-merge checks: 5

Check name	Status	Explanation
Description check		The description clearly explains the cached pepper fallback issue, the fail-closed behavior, the regression tests, and the reported validation results. It is directly related to the changeset.
Title check		The title clearly and concisely identifies the main change: authentication now fails closed when the cached pepper is explicitly blanked.
Docstring Coverage		Docstring coverage is inconclusive: files from earlier reviews still need to be rechecked within the check's analysis limits. Not blocking.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[PetrefiedThunder]

REVIEW: PASS claude 7814d3e

Verdict

PASS — fail-closed pepper sourcing when API_KEY_HASH_PEPPER is present but blank/whitespace; matches PR intent; tests green.

Scope vs description

• Exactly two files (api_key_service.py, test_api_key_scrypt.py), +24/−5.
• Two-file diff SHA256 29628d2bf4c4bada1735076041f5134c0823ff288185d5dd3bf51c32146b3988 matches Muse claim.
• No schema / public API / key-format / deploy surface change.

Correctness

• Pre-fix(api-keys)!: use salted, peppered scrypt digests #254-style bug: os.environ.get(..., "").strip() treated empty/whitespace as “missing,” then fell through to get_settings(), which could revive a cached nonempty SecretStr after Settings was primed with a real pepper.
• Fix: presence in os.environ is authoritative; Settings only when the variable is unset. Blank/" " → strip → None → production-like _api_key_hash_pepper() fail-closed → APIKeyHashingUnavailable.
• Absense still uses Settings (documented). Existing dotenv regression test remains valid under the new branch.

Security

• Tightens fail-closed auth config; does not weaken hashing, logging, or secret handling. Pepper still never logged; Settings remains SecretStr.
• No new attack surface; incorrect blank env now refuses boot/hashing instead of silently using a stale pepper.

Tests

• Ran uv run --with-requirements requirements-dev.txt pytest tests/test_api_key_scrypt.py -v on fresh Extreme SSD worktree at this SHA: 36 passed in ~4.6s (includes both parametrized cached-Settings regressions for "" and " ").

Residual / non-blocking

• Runtime delenv after Settings cache still falls back to Settings (by design per PR). Not a regression from this change.
• Merge still gated on CI green as noted in the PR body.

[PetrefiedThunder]

REVIEW: PASS claude 45dd80b

Delta vs PASS at 7814d3e

PASS — catch-up merges only; PR-side pepper fix unchanged; no conflict with #242.

PR-side identity

• Only two commits since 7814d3e: cb3c05d and 45dd80b (both Merge origin/main…). No new PR commits.
• gateway/app/services/api_key_service.py and gateway/tests/test_api_key_scrypt.py are byte-identical to 7814d3e (0-byte diffs).
• origin/main...45dd80b still +24/−5 on those two files; patch SHA256 still 29628d2bf4c4bada1735076041f5134c0823ff288185d5dd3bf51c32146b3988.
• Both merges: no conflict markers; first-parent deltas on the two PR files are empty (main did not rewrite them).

#242 interplay

• fix(security): fixed public MCP JSON-RPC errors + server-side logging with request id (code-scanning #2-#9) #242 (f29c962) is MCP JSON-RPC/REST error sanitization + server-side request-id logging (mcp.py + related tests). No overlap with api_key_service / pepper / scrypt paths; mcp.py has no refs to API_KEY_HASH_PEPPER, _configured_pepper, or api_key_service.
• Fail-closed pepper semantics unaffected by fixed public MCP error surfaces.

Tests (fresh Extreme SSD worktree at this SHA)

• tests/test_api_key_scrypt.py — 36 passed
• fix(security): fixed public MCP JSON-RPC errors + server-side logging with request id (code-scanning #2-#9) #242-touched: test_mcp_error_exposure_alerts_2_9.py + test_mcp_internal_error_disclosure.py — 35 passed
• 71 passed total in ~6s

Prior full-review findings at 7814d3e still hold. Merge still gated on CI green.