fix(security): fail closed without attestations

[Sarvesh Agarwal (sarvesh1327)]

Summary

• Fail closed during interceptor startup when the crypto attestation dependencies are unavailable.
• Refuse to return normal verdicts if JWT attestation signing fails, preventing attestation_jwt=None from being emitted silently.
• Update the README to document fail-closed attestations instead of graceful degradation, and add regression coverage for both paths.

Test Plan

• PYTHONPATH=src python3 -m pytest tests/test_interceptor.py -q
• PYTHONPATH=src python3 -m pytest tests/ -q
• PYTHONPATH=src python3 -m compileall -q src tests
• python3 -m ruff check src/qwed_a2a/interceptor.py tests/test_interceptor.py

Note: python3 -m ruff check . still reports pre-existing unused imports in untouched files (security/crypto.py, security/trust_boundary.py, and tests/test_crypto_signing.py).

Closes #7

Summary by CodeRabbit

• Bug Fixes

  • Attestation handling now fails closed: missing crypto support or any signing error prevents startup or emitting unsigned verdicts.

• Documentation

  • Clarified guidance: cryptography and JWT packages are required at startup; attestations are mandatory and signing failures are not degraded into normal verdicts.

• Tests

  • Added tests verifying startup failure when crypto is unavailable and that signing errors or empty tokens cause errors rather than returning unsigned verdicts.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3e84a39b-b241-4647-b293-603a75453243

📥 Commits

Reviewing files that changed from the base of the PR and between 7081614 and aa0aeeb.

📒 Files selected for processing (2)

• src/qwed_a2a/interceptor.py
• tests/test_interceptor.py

---

📝 Walkthrough

Hidden review stack artifact:

Walkthrough

The PR converts JWT attestation from optional (gracefully disabled when dependencies are missing) to mandatory fail-closed enforcement. The interceptor now raises at startup if cryptography or PyJWT are unavailable, and refuses to emit verdicts when attestation signing fails, ensuring every verdict is cryptographically signed.

Changes

JWT Attestation Fail-Closed Enforcement

Layer / File(s)	Summary
Fail-closed attestation requirement in interceptor src/qwed_a2a/interceptor.py	Class doc clarified to state verdict signing uses JWT attestation. __init__ raises RuntimeError instead of gracefully degrading when crypto dependencies are unavailable. _build_verdict always attempts signing and raises RuntimeError on signing exceptions or if sign_verdict returns an empty/falsy token.
Test coverage for fail-closed attestation behavior tests/test_interceptor.py	Three new async tests: constructor fails when HAS_CRYPTO is false; intercept() raises when crypto_service.sign_verdict raises; intercept() raises when sign_verdict returns an empty/falsy token.
Documentation update README.md	README section on crypto attestations replaced to document startup requirement for cryptography/PyJWT and removal of graceful degradation mode (attestations are fail-closed).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• QWED-AI/qwed-a2a#2: Directly addresses issue #7 by implementing fail-closed JWT attestation enforcement in the interceptor instead of graceful degradation.

Suggested labels

bug, fail-closed, p0

Poem

🐇 In the burrow of code I hop and see,
If crypto's gone, no verdict slips free.
I sign with care, no unsigned allowed,
Fail-closed we stand, and say it loud.
Hop, hop—secure by decree!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(security): fail closed without attestations' accurately describes the main change: enforcing fail-closed behavior when crypto attestation dependencies are unavailable.
Linked Issues check	✅ Passed	All primary requirements from issue #7 are met: interceptor raises RuntimeError on missing crypto deps, prevents returning verdicts with attestation_jwt=None, updates README documentation, and adds comprehensive failure-path tests.
Out of Scope Changes check	✅ Passed	All changes directly align with issue #7 objectives: interceptor fail-closed enforcement, README documentation updates, and test additions for missing-deps and signing-failure paths.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7081614485

ℹ️ 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 (@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 (@codex) address that feedback".

[chatgpt-codex-connector]

Validate signer output before returning a verdict

The fail-closed change still accepts any non-exception return from sign_verdict, so an injected crypto_service that returns None or an empty string on soft failure will emit a normal VerificationVerdict with a missing/invalid attestation. This directly violates the new “never emit unsigned verdicts” guarantee and can happen in real deployments that provide a custom signer wrapper. Treat falsy/invalid token output as a signing failure and raise instead of returning the verdict.

Useful? React with 👍 / 👎.

[Rahul Dass (rahuldass19)]

Hey Sarvesh Agarwal (@sarvesh1327) , nice work on this....the startup RuntimeError and the signing
failure path are exactly what was needed here. The default_allow=False fix
is a good catch too.

One thing before we merge: the Codex suggestion is still open. Right now
_build_verdict raises if sign_verdict throws an exception......but if someone
passes a custom crypto_service that returns None or an empty string without
raising, the code won't catch that and will still emit a verdict with no JWT.

Can you add a check after the sign_verdict call?

if not attestation_jwt:
    raise RuntimeError(
        "sign_verdict returned an empty token — refusing to emit unsigned verdict"
    )

And a test for it......something like your FailingCryptoService but returning
None instead of raising. Should be quick to add.

Once that's in, this is good to go.

[sentry]

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

[Rahul Dass (rahuldass19)]

Thanks for the contribution Sarvesh Agarwal (@sarvesh1327), especially the follow-up fix for empty attestation tokens and the regression coverage. The fail-closed path is much stronger now.

And yeah, the CI workflow conflict was actually on our side since we restructured Sonar/Snyk workflows after your fork/PR branch was already created, so no worries there.

Merged.

Feel free to contribute again in future if something in the QWED ecosystem aligns with your interests, especially around agent security, trust boundaries, verification semantics, or infra hardening.

Thank you so much again.