Where: src/middleware/apiKeyAuth.ts line ~25, and
src/graphql/schema.ts createGraphQLContext() line ~87.
What's wrong: both files independently implement the same "is this the
admin key" check:
const adminKey = process.env.ADMIN_API_KEY;
if (adminKey && providedKey === adminKey) { ... }This is a plain, non-constant-time string comparison. The codebase already
has src/lib/timing-safe.ts (timingSafeCompare) specifically to avoid this
class of bug, and src/routes/admin.ts correctly uses it for its own admin
bearer-token check:
if (!timingSafeCompare(authorization, `Bearer ${apiKey}`)) { ... }but that fix was never applied to apiKeyAuth.ts or the GraphQL context
builder, which duplicate the same admin-key check logic independently (see
companion issue about apiKeyAuth.ts being unused — the GraphQL copy is
live, since createGraphQLContext is wired into the /graphql endpoint).
Impact: the /graphql endpoint's admin check (isAdmin) is vulnerable to
a timing side-channel on ADMIN_API_KEY, the same class of issue previously
fixed in routes/admin.ts (issue #252 "Use timing-safe comparison for Bearer
token validation") but not applied consistently across all three places this
comparison now exists.
Suggested fix: replace both occurrences with timingSafeCompare (or
better, extract the "resolve isAdmin/isConsumer from headers" logic into one
shared helper used by both the middleware and the GraphQL context, since it's
currently copy-pasted almost verbatim between the two files).
Where:
src/middleware/apiKeyAuth.tsline ~25, andsrc/graphql/schema.tscreateGraphQLContext()line ~87.What's wrong: both files independently implement the same "is this the
admin key" check:
This is a plain, non-constant-time string comparison. The codebase already
has
src/lib/timing-safe.ts(timingSafeCompare) specifically to avoid thisclass of bug, and
src/routes/admin.tscorrectly uses it for its own adminbearer-token check:
but that fix was never applied to
apiKeyAuth.tsor the GraphQL contextbuilder, which duplicate the same admin-key check logic independently (see
companion issue about
apiKeyAuth.tsbeing unused — the GraphQL copy islive, since
createGraphQLContextis wired into the/graphqlendpoint).Impact: the
/graphqlendpoint's admin check (isAdmin) is vulnerable toa timing side-channel on
ADMIN_API_KEY, the same class of issue previouslyfixed in
routes/admin.ts(issue #252 "Use timing-safe comparison for Bearertoken validation") but not applied consistently across all three places this
comparison now exists.
Suggested fix: replace both occurrences with
timingSafeCompare(orbetter, extract the "resolve isAdmin/isConsumer from headers" logic into one
shared helper used by both the middleware and the GraphQL context, since it's
currently copy-pasted almost verbatim between the two files).