Skip to content

Protect affiliate settlement cron route#158

Merged
ralyodio merged 1 commit into
profullstack:masterfrom
morganschp:fix-affiliate-settle-cron-secret
May 23, 2026
Merged

Protect affiliate settlement cron route#158
ralyodio merged 1 commit into
profullstack:masterfrom
morganschp:fix-affiliate-settle-cron-secret

Conversation

@morganschp
Copy link
Copy Markdown
Contributor

Summary

  • fail closed when POST /api/affiliates/settle is deployed without CRON_SECRET
  • keep bearer-token auth required before creating the service client or settling commissions
  • add route regression coverage for missing config, missing bearer auth, and the authorized path

Fixes #156

Verification

  • corepack pnpm vitest run src/app/api/affiliates/settle/route.test.ts
  • corepack pnpm type-check
  • corepack pnpm prettier --check src/app/api/affiliates/settle/route.ts src/app/api/affiliates/settle/route.test.ts
  • git diff --check

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 23, 2026

Greptile Summary

This PR fixes a fail-open security bug in the affiliate settlement cron route: the previous guard if (cronSecret && authHeader !== ...) would silently pass all requests when CRON_SECRET was unset, allowing unauthenticated commission settlement. The fix splits the check into two explicit gates and adds regression test coverage for the three critical paths.

  • route.ts: Replaces the single compound conditional with two ordered checks — first explicitly rejecting when CRON_SECRET is absent (fail-closed), then checking the bearer token. The service client is only instantiated after both guards pass.
  • route.test.ts: New test file covering missing config, missing auth header, and the authorized happy path, with mocks for createServiceClient and settleCommissions to assert neither side-effect fires on rejected requests.

Confidence Score: 5/5

Safe to merge — the change correctly closes a fail-open auth gap and is backed by targeted regression tests.

The two-gate auth refactor is minimal, easy to reason about, and the tests directly cover all three rejection conditions. No prior authenticated path is removed; the only behavioral change is that a misconfigured deployment now returns 401 instead of processing requests without any secret check.

No files require special attention.

Important Files Changed

Filename Overview
src/app/api/affiliates/settle/route.ts Fixes fail-open auth: splits compound cronSecret && guard into two explicit checks so missing CRON_SECRET now correctly rejects with 401 instead of allowing unauthenticated access.
src/app/api/affiliates/settle/route.test.ts New regression test suite covering all three auth paths (missing config, missing bearer token, authorized); correctly uses vi.hoisted and vi.stubEnv with beforeEach reset to avoid state leakage between tests.

Sequence Diagram

sequenceDiagram
    participant Cron as Cron Job / Caller
    participant Route as POST /api/affiliates/settle
    participant Env as process.env
    participant SC as createServiceClient
    participant Settle as settleCommissions

    Cron->>Route: POST (Authorization: Bearer secret)
    Route->>Env: read CRON_SECRET
    alt CRON_SECRET missing or empty
        Route-->>Cron: 401 Unauthorized (fail-closed)
    else CRON_SECRET present
        Route->>Route: "compare authHeader to Bearer {cronSecret}"
        alt Token mismatch or missing
            Route-->>Cron: 401 Unauthorized
        else Token matches
            Route->>SC: createServiceClient()
            SC-->>Route: admin client
            Route->>Settle: "settleCommissions(admin, { limit: 100 })"
            Settle-->>Route: "{ settled, failed, total_sats }"
            Route-->>Cron: "200 { settled, failed, total_sats }"
        end
    end
Loading

Reviews (3): Last reviewed commit: "Protect affiliate settlement cron route" | Re-trigger Greptile

return NextResponse.json({ error: "Unauthorized" }, { status: 401 });
}

if (authHeader !== `Bearer ${cronSecret}`) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Timing-sensitive bearer token comparison

The check authHeader !== \Bearer ${cronSecret}`uses a non-constant-time string comparison. An attacker who can make many rapid requests and measure response latency can theoretically brute-force the secret one character at a time. In practice this risk is low for a cron endpoint, but for secrets guarding financial settlement operations it is worth switching to a timing-safe compare such as Node'scrypto.timingSafeEqualor thesafe-compare` package to eliminate the side-channel entirely.

@ralyodio ralyodio closed this May 23, 2026
@ralyodio ralyodio reopened this May 23, 2026
@ralyodio ralyodio closed this May 23, 2026
@ralyodio ralyodio reopened this May 23, 2026
@ralyodio ralyodio merged commit a5f73ac into profullstack:master May 23, 2026
4 checks passed
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.

bug: affiliate settle route allows requests when CRON_SECRET is unset

2 participants