test(security): add webhook auth and token management tests#499
test(security): add webhook auth and token management tests#499marcoscaceres wants to merge 5 commits intomainfrom
Conversation
marcoscaceres
force-pushed
the
test/security
branch
from
5fc595f to
97d9047
Compare
There was a problem hiding this comment.
Pull request overview
This PR strengthens security around GitHub integrations by hardening webhook signature verification and improving GitHub token masking, with new Jasmine specs to lock in the behavior.
Changes:
- Updated webhook signature validation to use
crypto.timingSafeEqual()instead of===for comparison. - Updated token masking in
getLimits()to hide all but the last 4 characters. - Added new Jasmine test suites covering webhook auth and token masking/rate-limit storage behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| utils/auth-github-webhook.ts | Switches signature comparison to timingSafeEqual() and adds basic header/length checks. |
| tests/utils/auth-github-webhook.test.js | Adds webhook authenticator/verifier tests covering valid/invalid signatures and ping handling. |
| routes/github/lib/utils/tokens.ts | Changes token masking logic used by getLimits() to avoid leaking token prefixes. |
| tests/routes/github/lib/utils/tokens.test.js | Adds tests for getToken(), updateRateLimit(), and token masking behavior in getLimits(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
marcoscaceres
force-pushed
the
test/security
branch
from
97d9047 to
c3aec42
Compare
There was a problem hiding this comment.
Pull request overview
Adds automated test coverage for GitHub webhook HMAC authentication and GitHub token masking, while hardening signature verification to mitigate timing attacks and reducing token leakage in rate-limit reporting.
Changes:
- Update webhook signature verification to use
timingSafeEqualinstead of===. - Add Jasmine specs covering webhook verification behavior (valid/invalid/missing signatures and ping events).
- Fix token masking in
getLimits()to hide all but the last 4 characters, and add specs validating non-leakage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| utils/auth-github-webhook.ts | Switches webhook signature comparison to a constant-time primitive. |
| routes/github/lib/utils/tokens.ts | Updates token masking logic to avoid leaking token prefixes. |
| tests/utils/auth-github-webhook.test.js | Adds webhook authenticator and verifier behavior tests. |
| tests/routes/github/lib/utils/tokens.test.js | Adds tests for token retrieval, rate-limit storage, and masking behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR strengthens webhook authentication and GitHub token handling by adding targeted security-focused tests and hardening comparisons/masking to avoid timing attacks and token leakage.
Changes:
- Switch GitHub webhook signature verification to a constant-time comparison (
timingSafeEqual) and add comprehensive verifier middleware specs. - Fix token masking in
getLimits()to avoid leaking token prefixes and add tests covering masking/rotation/rate-limit updates.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| utils/auth-github-webhook.ts | Uses timingSafeEqual for signature verification and adds missing-header handling. |
| tests/utils/auth-github-webhook.test.js | Adds a new test suite covering webhook verifier behavior and failure modes. |
| routes/github/lib/utils/tokens.ts | Updates token masking to hide all but the last 4 characters (or fully mask short tokens). |
| tests/routes/github/lib/utils/tokens.test.js | Adds tests for token retrieval, rate-limit tracking, and masked-key behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
marcoscaceres
force-pushed
the
test/security
branch
from
c3aec42 to
0941048
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens GitHub-related security utilities by making webhook signature verification constant-time and by ensuring GitHub tokens are properly masked when surfaced via rate-limit reporting, with accompanying Jasmine specs.
Changes:
- Use
crypto.timingSafeEqual()for GitHub webhook HMAC signature comparisons. - Add a new test suite covering webhook authentication behavior and edge cases.
- Fix token masking in
getLimits()to avoid leaking token prefixes, with tests for masking/cycling and rate-limit updates.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| utils/auth-github-webhook.ts | Switches signature check to constant-time comparison and adds basic header presence/length guards. |
| tests/utils/auth-github-webhook.test.js | Adds webhook authenticator specs (valid/invalid signature, ping handling, tampering, missing header). |
| routes/github/lib/utils/tokens.ts | Updates token masking logic to hide all but the last 4 characters (or all for very short tokens). |
| tests/routes/github/lib/utils/tokens.test.js | Adds specs for token cycling, rate-limit storage, and masking guarantees (no full token/prefix exposure). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR strengthens security around GitHub webhook authentication and GitHub token handling, and adds regression tests to cover signature verification and token masking/rate-limit bookkeeping.
Changes:
- Use constant-time comparison (
timingSafeEqual) for webhook HMAC signature verification. - Add a dedicated test suite covering webhook signature verification behavior (valid/invalid/missing signatures and ping events).
- Fix token masking in
getLimits()to avoid leaking token prefixes and add tests for masking + rate-limit storage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| utils/auth-github-webhook.ts | Replaces === signature check with timingSafeEqual for webhook auth. |
| tests/utils/auth-github-webhook.test.js | Adds webhook authenticator specs (signature validity, tampering, missing header, ping). |
| routes/github/lib/utils/tokens.ts | Updates secureToken() masking logic to hide everything except the last 4 chars. |
| tests/routes/github/lib/utils/tokens.test.js | Adds specs for token retrieval/rotation, rate-limit updates, and masking guarantees. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Adds test coverage for GitHub webhook authentication and GitHub token masking/rate-limit bookkeeping, alongside security hardening of signature comparison and token redaction behavior.
Changes:
- Switch webhook signature verification from
===totimingSafeEqualto reduce timing-attack risk. - Add a new Jasmine suite covering webhook signature verification outcomes (valid/invalid/missing/tampered/ping).
- Update token masking in
getLimits()to avoid leaking token prefixes; add tests for masking and rate-limit storage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| utils/auth-github-webhook.ts | Harden signature comparison using constant-time equality. |
| tests/utils/auth-github-webhook.test.js | Add webhook authenticator verification tests using request/response mocks. |
| routes/github/lib/utils/tokens.ts | Fix token masking to reveal at most the last 4 characters. |
| tests/routes/github/lib/utils/tokens.test.js | Add tests for token retrieval/rotation, rate-limit updates, and masking properties. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR strengthens GitHub integration security by hardening webhook signature verification against timing attacks and improving GitHub token masking, with new Jasmine specs to validate both behaviors.
Changes:
- Replace
===signature comparison withtimingSafeEqualin GitHub webhook authentication. - Update token masking to hide everything except the last 4 characters.
- Add new test suites covering webhook auth behavior and token masking/rate-limit tracking.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
utils/auth-github-webhook.ts |
Uses timingSafeEqual for webhook signature verification. |
tests/utils/auth-github-webhook.test.js |
Adds specs for webhook signature validation and ping handling. |
routes/github/lib/utils/tokens.ts |
Fixes token masking to avoid leaking token prefixes. |
tests/routes/github/lib/utils/tokens.test.js |
Adds specs for token cycling, rate-limit storage, and masking guarantees. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR strengthens security around GitHub integrations by hardening webhook signature verification against timing attacks and by preventing GitHub token prefix leakage when exposing rate-limit state.
Changes:
- Replace direct string comparison of webhook signatures with
timingSafeEqualinisValidGithubSignature(). - Add a new Jasmine test suite covering webhook verifier behaviors (valid/invalid/missing signature, ping event, tampered body, etc.).
- Fix token masking in
getLimits()to reveal at most the last 4 characters (and add tests validating masking behavior).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| utils/auth-github-webhook.ts | Switches signature comparison to constant-time comparison. |
| tests/utils/auth-github-webhook.test.js | Adds webhook authentication/verifier tests. |
| routes/github/lib/utils/tokens.ts | Fixes token masking to avoid leaking token prefix. |
| tests/routes/github/lib/utils/tokens.test.js | Adds tests for token retrieval, rate-limit updates, and masking behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add comprehensive test suites for two security-critical modules: - utils/auth-github-webhook: 8 tests covering valid HMAC verification, invalid/missing/empty signatures, body tampering, and wrong secrets. - routes/github/lib/utils/tokens: 9 tests covering token cycling, rate limit tracking, and token masking in getLimits() output. Fix two security issues discovered during testing: 1. Webhook signature comparison used === (timing-attack vulnerable). Now uses crypto.timingSafeEqual() with explicit length check and early return for missing headers. 2. Token masking in getLimits() replaced only the last 30 chars with 2 asterisks, leaking up to 10 prefix characters of a 40-char GitHub token. Now masks all but the last 4 characters with per-character asterisks, preserving output length.
marcoscaceres
force-pushed
the
test/security
branch
from
0941048 to
cdd787d
Compare
There was a problem hiding this comment.
Pull request overview
This PR strengthens GitHub-related security behavior and adds regression tests to ensure webhook HMAC verification is constant-time and GitHub token masking no longer leaks sensitive prefixes.
Changes:
- Updated GitHub webhook signature verification to use
timingSafeEqualwith explicit length checks. - Added a new webhook authenticator test suite covering valid/invalid/missing/tampered signatures and ping handling.
- Updated token masking in
getLimits()and added tests to ensure only the last 4 characters are ever exposed.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| utils/auth-github-webhook.ts | Replaces === signature comparison with timingSafeEqual and length checks for safer HMAC verification. |
| tests/utils/auth-github-webhook.test.js | Adds comprehensive unit tests for the webhook authenticator middleware behavior. |
| routes/github/lib/utils/tokens.ts | Fixes token masking to hide all but the last 4 characters (or fully mask very short tokens). |
| tests/routes/github/lib/utils/tokens.test.js | Adds tests verifying token retrieval, rate limit storage, and strict masking/no-prefix leakage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…tokens
Fix non-ASCII signature test to pass { body, headers } object
instead of positional args. Add explicit test for wrong-length
X-Hub-Signature. Guard masking test against GH_TOKEN shorter
than 4 characters.
There was a problem hiding this comment.
Pull request overview
This PR strengthens GitHub integration security by making webhook signature verification resistant to timing attacks and ensuring GitHub tokens are properly masked, with new Jasmine specs covering both areas.
Changes:
- Use
timingSafeEqualfor webhook HMAC signature comparison instead of===. - Fix token masking in
getLimits()to avoid leaking token prefixes (mask all but last 4 chars). - Add new test suites for webhook auth behavior and GitHub token masking/rate-limit tracking.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| utils/auth-github-webhook.ts | Switches signature comparison to timingSafeEqual with length guarding. |
| routes/github/lib/utils/tokens.ts | Updates secureToken() masking logic to prevent token-prefix leakage. |
| tests/utils/auth-github-webhook.test.js | Adds webhook signature verification test coverage (valid/invalid/missing/tampered/ping, etc.). |
| tests/routes/github/lib/utils/tokens.test.js | Adds tests for token retrieval, rate limit updates, and masking guarantees. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@copilot please resolve and fix any unaddressed comment |
Done in 553b29d. Addressed the two remaining unresolved threads:
All 62 tests still pass. |
|
@copilot Can you review this and fix any issues that come up? |
Summary
Closes #384