Skip to content

Add sinon/rewire unit tests for OCSP verification#490

Closed
claude[bot] wants to merge 2 commits intomainfrom
claude/fix-119
Closed

Add sinon/rewire unit tests for OCSP verification#490
claude[bot] wants to merge 2 commits intomainfrom
claude/fix-119

Conversation

@claude
Copy link
Copy Markdown
Contributor

@claude claude Bot commented May 7, 2026

Summary

  • Adds performOCSPCheck() tests using rewire to mock the getCertStatus export from easy-ocsp (which has non-configurable property descriptors, so sinon.stub can't be used directly). Covers good, revoked (with and without reason), unknown, AbortError/timeout, network-error, timeout forwarding, and OCSP URL passthrough.
  • Adds verifyOCSP() tests that clear the require cache per test and stub getCertificateCacheTable on verificationUtils so each test gets an isolated cache mock. Covers cache-hit good/revoked, cached flag (hit vs source-fetch), null-cache fail-closed/fail-open, and context passthrough to the cache get() call.

Existing tests (smoke-test style with invalid certs) are unchanged.

Test plan

  • npm run test:unit:security — 325 passing (up from 310), 1 pending, 2 pre-existing failures unrelated to cert verification

Closes #119

🤖 Generated with Claude Code

…ncies

Cover the specific scenarios requested in #119 that were missing from the
existing smoke-test style coverage:

- `performOCSPCheck()`: use rewire to inject a mock for `getCertStatus` from
  easy-ocsp (which has non-configurable exports) and assert good, revoked,
  unknown, AbortError/timeout, network-error, timeout forwarding, and OCSP
  URL passthrough paths.

- `verifyOCSP()`: clear require cache per test and stub
  `getCertificateCacheTable` on verificationUtils so each test gets an
  isolated cache table mock; covers cache-hit good/revoked, cached flag
  (hit vs source-fetch), null-cache fail-closed/fail-open, and context
  passthrough.

Closes #119

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude claude Bot requested a review from a team as a code owner May 7, 2026 14:44
@claude
Copy link
Copy Markdown
Contributor Author

claude Bot commented May 7, 2026

Reviewed; no blockers found.

@heskew
Copy link
Copy Markdown
Member

heskew commented May 7, 2026

@claude run npm run format:write to fix the prettier issues in unitTests/security/certificateVerification/ocspVerification.test.js, then commit + push. Format-only change, no other edits.

Fixes formatting issues flagged by prettier in PR #490.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@heskew
Copy link
Copy Markdown
Member

heskew commented May 7, 2026

Closing + reopening to force CI retrigger — Claude's format-fix commit was authored by github-actions[bot], and GITHUB_TOKEN-pushed commits don't trigger downstream workflows (deliberate loop-prevention guard). Reopening will fire pull_request.reopened on the current HEAD.

@heskew heskew closed this May 7, 2026
@heskew heskew reopened this May 7, 2026
Copy link
Copy Markdown
Member

@kriszyp kriszyp left a comment

Choose a reason for hiding this comment

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

I thought we were trying to reduce our usage of rewire (incompatible with TypeStrip), but whatever, hard to turn down more tests 🤷 .

@heskew
Copy link
Copy Markdown
Member

heskew commented May 7, 2026

I thought we were trying to reduce our usage of rewire (incompatible with TypeStrip), but whatever, hard to turn down more tests 🤷 .

Yep. I need to tell claude. It saw what had been used elsewhere and went with it. I'll have it re-try after some workflow update(s).

@heskew
Copy link
Copy Markdown
Member

heskew commented May 7, 2026

@kriszyp #491

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.

Add unit tests for OCSP and CRL certificate verification

2 participants