Skip to content

Commit cdd787d

Browse files
committed
test(security): add webhook auth and token management tests
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.
1 parent 5916d39 commit cdd787d

4 files changed

Lines changed: 401 additions & 4 deletions

File tree

routes/github/lib/utils/tokens.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ export function updateRateLimit(token: string, rateLimit: RateLimit) {
2626
}
2727

2828
export function getLimits() {
29-
const secureToken = (token: string) => token.replace(/.{30}$/, "*".repeat(2));
29+
const secureToken = (token: string) =>
30+
token.length <= 4
31+
? "*".repeat(token.length)
32+
: "*".repeat(token.length - 4) + token.slice(-4);
3033
const result: Record<string, RateLimit | null> = {};
3134
for (const [token, limits] of LIMITS) {
3235
result[secureToken(token)] = limits;
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
// GH_TOKEN is set by tests/helpers/env.js before any module loads.
2+
// The tokens module reads it at import time, so we use whatever value
3+
// the helper provided.
4+
const {
5+
getToken,
6+
updateRateLimit,
7+
getLimits,
8+
} = await import(
9+
"../../../../../build/routes/github/lib/utils/tokens.js"
10+
);
11+
12+
describe("routes/github/lib/utils/tokens", () => {
13+
describe("getToken()", () => {
14+
it("returns a token string", () => {
15+
const token = getToken();
16+
expect(typeof token).toBe("string");
17+
expect(token.length).toBeGreaterThan(0);
18+
});
19+
20+
it("returns the configured GH_TOKEN", () => {
21+
const token = getToken();
22+
expect(token).toBe(process.env.GH_TOKEN);
23+
});
24+
25+
it("cycles back to the same token (single-token rotation)", () => {
26+
// With a single token, every call returns the same value
27+
const first = getToken();
28+
const second = getToken();
29+
const third = getToken();
30+
expect(first).toBe(second);
31+
expect(second).toBe(third);
32+
});
33+
});
34+
35+
describe("updateRateLimit()", () => {
36+
it("stores rate limit data for a token", () => {
37+
const token = getToken();
38+
const rateLimit = {
39+
remaining: 4500,
40+
resetAt: new Date("2026-01-01T00:00:00Z"),
41+
limit: 5000,
42+
};
43+
updateRateLimit(token, rateLimit);
44+
45+
const limits = getLimits();
46+
const entries = Object.values(limits);
47+
// The stored rate limit should match what we set
48+
const stored = entries.find(v => v !== null);
49+
expect(stored).toBeDefined();
50+
expect(stored?.remaining).toBe(4500);
51+
expect(stored?.limit).toBe(5000);
52+
});
53+
54+
it("updates existing rate limit data", () => {
55+
const token = getToken();
56+
const first = {
57+
remaining: 4500,
58+
resetAt: new Date("2026-01-01T00:00:00Z"),
59+
limit: 5000,
60+
};
61+
updateRateLimit(token, first);
62+
63+
const second = {
64+
remaining: 100,
65+
resetAt: new Date("2026-01-01T01:00:00Z"),
66+
limit: 5000,
67+
};
68+
updateRateLimit(token, second);
69+
70+
const limits = getLimits();
71+
const entries = Object.values(limits);
72+
const stored = entries.find(v => v !== null);
73+
expect(stored.remaining).toBe(100);
74+
});
75+
});
76+
77+
describe("getLimits()", () => {
78+
it("returns an object keyed by masked tokens", () => {
79+
const limits = getLimits();
80+
expect(typeof limits).toBe("object");
81+
expect(Object.keys(limits).length).toBeGreaterThan(0);
82+
});
83+
84+
it("never exposes the full token in keys", () => {
85+
const fullToken = process.env.GH_TOKEN;
86+
const limits = getLimits();
87+
for (const key of Object.keys(limits)) {
88+
expect(key).not.toBe(fullToken);
89+
}
90+
});
91+
92+
it("masks all but the last 4 characters of the token", () => {
93+
const fullToken = process.env.GH_TOKEN;
94+
const limits = getLimits();
95+
const keys = Object.keys(limits);
96+
expect(keys).toHaveSize(1);
97+
98+
const maskedKey = keys[0];
99+
// Should show only the last 4 characters, rest are asterisks
100+
const expectedSuffix = fullToken.slice(-4);
101+
const expectedStars = "*".repeat(fullToken.length - 4);
102+
expect(maskedKey).toBe(expectedStars + expectedSuffix);
103+
});
104+
105+
it("masked key has the same length as the full token", () => {
106+
const fullToken = process.env.GH_TOKEN;
107+
const limits = getLimits();
108+
const maskedKey = Object.keys(limits)[0];
109+
// Length is preserved (stars replace chars 1:1)
110+
expect(maskedKey.length).toBe(fullToken.length);
111+
});
112+
113+
it("does not leak the token prefix (ghp_ or similar)", () => {
114+
const fullToken = process.env.GH_TOKEN;
115+
const limits = getLimits();
116+
const maskedKey = Object.keys(limits)[0];
117+
// The ghp_ prefix must NOT appear in the masked key
118+
const prefix = fullToken.slice(0, 4);
119+
expect(maskedKey.startsWith(prefix)).toBeFalse();
120+
expect(maskedKey.startsWith("*")).toBeTrue();
121+
});
122+
123+
it("exposes at most 4 characters of the actual token", () => {
124+
const fullToken = process.env.GH_TOKEN;
125+
const limits = getLimits();
126+
const maskedKey = Object.keys(limits)[0];
127+
// Count non-asterisk characters
128+
const visibleChars = [...maskedKey].filter(c => c !== "*").length;
129+
expect(visibleChars).toBeLessThanOrEqual(4);
130+
});
131+
});
132+
});

0 commit comments

Comments
 (0)