Skip to content

Fix/1746 admin auth - #1755

Merged
Baskarayelu merged 4 commits into
Remitwise-Org:mainfrom
emarkees:fix/1746-admin-auth
Oct 22, 2026
Merged

Baskarayelu merged 4 commits into
Remitwise-Org:mainfrom
emarkees:fix/1746-admin-auth

Conversation

@emarkees

Copy link
Copy Markdown

security(admin): implement constant-time header-only admin guard (#1746)

Closes #1746

Summary of Changes

Re-implemented @/lib/admin/auth (lib/admin/auth.ts) and its unit test suite (lib/admin/__tests__/auth.test.ts) to provide an administrative authentication guard for protected endpoints (such as app/api/cache/invalidate/route.ts).

The public interface matches the contract expected by consumer routes and existing integration tests:

  • isAdminAuthorized(request): boolean
  • getAdminIdentity(request): string

Security Problems in the Prior Implementation & How They Are Resolved

1. Cross-Site Request Forgery (CSRF)

  • Problem: The previous implementation accepted the admin secret via admin_key or admin_secret browser cookies. Browsers attach cookies automatically to cross-site requests. Consequently, an admin visiting an untrusted website could have a cross-origin form or script execute a POST /api/cache/invalidate request with {"clearAll": true}, wiping the contract cache.
  • Resolution: Credentials are exclusively extracted from the x-admin-key HTTP header. Cookies and URL query parameters are strictly ignored.

2. Length-Oracle Timing Attack

  • Problem: The deleted implementation called crypto.timingSafeEqual directly on the provided and expected secret byte buffers. When inputs differed in length, crypto.timingSafeEqual threw an immediate exception. Catching this mismatch and returning false introduced a measurable timing difference between wrong-length inputs (which failed instantly) and equal-length inputs (which proceeded to byte comparison), leaking the secret's character count to attackers.
  • Resolution: Both the provided header and the expected secret are first hashed into fixed-length HMAC digests using a 32-byte per-process random key (crypto.createHmac("sha256", PROCESS_HMAC_KEY)). Because HMAC-SHA256 digests are uniformly 32 bytes regardless of input length, crypto.timingSafeEqual always compares equal-length buffers in constant time without throwing exceptions.

3. Weak Secret and Whitespace Handling

  • Problem: The prior code allowed short secrets (e.g., "a") and called .trim() on the header input, silently ignoring whitespace discrepancies.
  • Resolution: Enforces that ADMIN_SECRET must be at least 32 characters long. If unset or shorter than 32 characters, all requests are rejected and a single startup console.error is logged naming the variable (ADMIN_SECRET) without leaking its value. The incoming x-admin-key header is evaluated as-is without .trim(), so any trailing or leading whitespace properly fails closed. Key rotation is supported via comma-separated secrets in ADMIN_SECRET.

4. Audit Attribution

  • Problem: Previously returned fixed labels (header:x-admin-key) regardless of which key was used, even if invalid.
  • Resolution: getAdminIdentity(req) returns admin:<hash> (where <hash> is the first 8 hex characters of the SHA-256 digest of the matched secret), providing a stable identifier across calls without disclosing the secret itself. Unauthenticated requests return 'anonymous'.

Comparison Code

const PROCESS_HMAC_KEY = crypto.randomBytes(32);

function constantTimeEqual(provided: string, expected: string): boolean {
  const hmacProvided = crypto
    .createHmac("sha256", PROCESS_HMAC_KEY)
    .update(provided)
    .digest();
  const hmacExpected = crypto
    .createHmac("sha256", PROCESS_HMAC_KEY)
    .update(expected)
    .digest();

  return crypto.timingSafeEqual(hmacProvided, hmacExpected);
}

@emarkees

emarkees commented Oct 19, 2026 •

Copy link
Copy Markdown
Author

@Baskarayelu Pls review and merge

@emarkees

emarkees commented Oct 19, 2026 •

Copy link
Copy Markdown
Author

@Baskarayelu conflict resolved and if you still get one more for, i will really appreciate to do one more contribution. Thanking you in advance

@emarkees

Copy link
Copy Markdown
Author

@Baskarayelu be mindful what you merge, I have a friend who's project was attacked with malware today, pls check carefully before you merge any contibutors code

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.

security(admin): admin guard for /api/cache/invalidate must be header-only, constant-time and fail closed

2 participants