Skip to content

fix(screener-api): enforce HMAC timestamp freshness on /api/data/sync - #4078

Open
SashaMIT wants to merge 1 commit into
synapsecns:masterfrom
SashaMIT:fix/screener-hmac-timestamp-freshness
Open

fix(screener-api): enforce HMAC timestamp freshness on /api/data/sync#4078
SashaMIT wants to merge 1 commit into
synapsecns:masterfrom
SashaMIT:fix/screener-hmac-timestamp-freshness

Conversation

@SashaMIT

@SashaMIT SashaMIT commented Aug 8, 2026

Copy link
Copy Markdown

Summary

POST /api/data/sync HMAC auth folds X-Signature-timestamp and X-Signature-nonce into the signed message, but the server never enforced timestamp freshness (or app id binding). A captured valid request could be replayed indefinitely against blacklist create/update/delete.

Change

  • Reject requests whose X-Signature-timestamp is outside a 5-minute skew window (or non-numeric).
  • Require X-Signature-appid to match configured AppID.
  • Compare signatures with hmac.Equal.

Nonce uniqueness is not persisted here (would need shared storage); freshness closes indefinite replay of old captures.

Test plan

  • GOWORK=off go test ./screener/ -run TestSignatureTimestampFresh → ok
  • CI green

Tip: ef5f972c4be446b74daf781b6a90b6198b7a4486

Made with Cursor

Summary by CodeRabbit

  • Security Enhancements
    • Added validation to reject requests with missing, invalid, stale, or future authentication timestamps.
    • Improved signature verification to help prevent timing-based attacks.
  • Tests
    • Added coverage for valid, expired, future, empty, and non-numeric timestamps.

authMiddleware included X-Signature-timestamp (and nonce) in the signed
message but never checked skew, so a captured blacklist create/update/delete
could be replayed indefinitely. Also bind X-Signature-appid to cfg.AppID and
compare digests with hmac.Equal.
@SashaMIT
SashaMIT requested a review from trajan0x as a code owner August 8, 2026 20:16
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

@SashaMIT is attempting to deploy a commit to the synapsecns Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added go Pull requests that update Go code size/s labels Aug 8, 2026
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ea7fd27a-c008-4530-a149-987f8241e4d5

📥 Commits

Reviewing files that changed from the base of the PR and between ef5f972 and 6c10418.

📒 Files selected for processing (2)
  • contrib/screener-api/screener/auth_middleware_test.go
  • contrib/screener-api/screener/screener.go

📝 Walkthrough

Walkthrough

The screener authentication middleware now validates signature timestamps within a five-minute skew and compares signatures in constant time. Table-driven tests cover valid, stale, future, empty, and non-numeric timestamps.

Changes

Signature authentication

Layer / File(s) Summary
Timestamp validation
contrib/screener-api/screener/screener.go, contrib/screener-api/screener/auth_middleware_test.go
The middleware parses Unix-second timestamps and accepts values within five minutes of the current time. Tests cover valid boundaries, stale and future values, empty input, and non-numeric input.
Middleware signature checks
contrib/screener-api/screener/screener.go
Authentication rejects missing or unequal app IDs and invalid or stale timestamps before reading the request body. Signature comparison uses constant-time HMAC comparison.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: trajan0x

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: enforcing HMAC timestamp freshness for the screener API endpoint.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code size/s

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant