AntFleet

Anatomy · be39e8a7-2

Authorization timing check leaks length oracle; comment claims otherwise

lowsecurityclosed in a58382a
repo e24ef98c·PR #10·reviewed 1 week ago·closed 1 week ago

The vulnerable code

apps/web/app/api/cron/sweep/route.ts:0-0

0import { timingSafeEqual } from "node:crypto";
1import type { NextRequest } from "next/server";
2import { NextResponse } from "next/server";
3import { logError, logInfo, logWarn, messageOf } from "@/lib/log";
4import { runDailyOnboarderCheckIns } from "@/lib/onboarder";
5import { runOutgoingPrsPoll } from "@/lib/outgoing-prs";
6import { runSweep } from "@/lib/sweep";
7
8// node:crypto + DB driver are Node-only — lock this off Edge.
9export const runtime = "nodejs";
10
11// Sweeper iterates every open finding sequentially against the GitHub
12// API — historically fast enough under the 60s Hobby ceiling. Onboarder
13// check-ins run on the same tick (slice O5); each candidate adds one
14// Anthropic call (~30s) plus a GitHub post. Bumped to 180s (Pro plan
15// has 300s headroom) so a single-digit-partner Phase 2 stays comfortable.
16export const maxDuration = 180;
17
18export async function GET(req: NextRequest): Promise<NextResponse> {
19 const secret = process.env["CRON_SECRET"];
20 if (secret === undefined || secret.length === 0) {
21 logError("cron.misconfigured", { reason: "CRON_SECRET missing" });
22 return new NextResponse("server misconfigured", { status: 500 });
23 }
24 const authHeader = req.headers.get("authorization");
25 // Vercel's cron invocation sets Authorization: Bearer <CRON_SECRET>.
26 // Constant-time compare to deny a length / prefix oracle to anything that
27 // can reach this route before Vercel's edge rate-limit (defense in depth).
28 const expected = `Bearer ${secret}`;
29 const provided = authHeader ?? "";
30 const a = Buffer.from(provided);
31 const b = Buffer.from(expected);
32 if (a.length !== b.length || !timingSafeEqual(a, b)) {
33 logWarn("cron.unauthorized", { hasAuth: authHeader !== null });
34 return new NextResponse("unauthorized", { status: 401 });
35 }
36
37 const t0 = Date.now();
38 try {
39 const result = await runSweep();
40 const sweepMs = Date.now() - t0;
41 logInfo("cron.sweep_complete", {
42 swept: result.swept,
43 closed: result.closed,
44 reactionsRecorded: result.reactionsRecorded,
45 reviewsSkipped: result.reviewsSkipped,
46 errorCount: result.errors.length,
47 elapsedMs: sweepMs,
48 });
49
50 // Onboarder daily check-in runs on the same cron tick as Sweeper.
51 // Self-gates on ONBOARDER_ENABLED so prod stays silent until flipped.
52 // Failure here is logged but does not 5xx the cron — Sweeper success
53 // is the primary load-bearing outcome of this tick.
54 const tOnboarder = Date.now();
55 let onboarderResult: Awaited<ReturnType<typeof runDailyOnboarderCheckIns>> = {
56 attempted: 0,
57 posted: 0,
58 skipped: 0,
59 errors: 0,
60 };
61 try {
62 onboarderResult = await runDailyOnboarderCheckIns(new Date());
63 logInfo("cron.onboarder_checkins_complete", {
64 ...onboarderResult,
65 elapsedMs: Date.now() - tOnboarder,
66 });
67 } catch (err) {
68 const message = messageOf(err);
69 logError("cron.onboarder_checkins_failed", { message });
70 }
71
72 // Cross-repo receipts — poll the merge state of outgoing PRs that
73 // antfleet-ops opened on third-party repos. Wraps its own try/catch
74 // internally (runOutgoingPrsPoll returns null on failure) so it can
75 // never destabilize the cron tick. Skipped silently in environments
76 // without ANTFLEET_OPS_GH_TOKEN — the lazy token read inside
77 // realPollDeps throws and is converted to a log line.
78 const tOutgoing = Date.now();
79 const outgoingResult = await runOutgoingPrsPoll();
80 if (outgoingResult !== null) {
81 logInfo("cron.outgoing_prs_poll_complete", {
82 ...outgoingResult,
83 elapsedMs: Date.now() - tOutgoing,
84 });
85 }
86
87 const elapsedMs = Date.now() - t0;
88 return NextResponse.json({
89 ...result,
90 onboarder: onboarderResult,
91 outgoingPrs: outgoingResult,
92 elapsedMs,
93 });
94 } catch (err) {
95 const message = messageOf(err);
96 logError("cron.sweep_failed", { message });
97 return new NextResponse("sweep failed", { status: 500 });
98 }
99}
100

The reasoning

Opus

Output unavailable for this row.

GPT-5

Authorization timing check leaks length oracle; comment claims otherwise

lowsecurityhigh
  • apps/web/app/api/cron/sweep/route.ts
The code returns early when lengths differ, which creates a measurable timing difference between length-mismatch and equal-length attempts. The nearby comment states the goal is to deny a length/prefix oracle; the current implementation still reveals the expected length. While mitigated by platform rate limiting, the code and comment diverge, and best practice is to keep timing as constant as possible.

Recommendation

Avoid early-return solely on length mismatch. Pad or truncate the provided buffer to the expected length and always call timingSafeEqual, then separately validate that the original lengths match before accepting. For example: - const b = Buffer.from(expected) - const aRaw = Buffer.from(provided) - const a = aRaw.length === b.length ? aRaw : Buffer.concat([aRaw.slice(0, b.length), Buffer.alloc(Math.max(0, b.length - aRaw.length))]) - const ok = timingSafeEqual(a, b) && aRaw.length === b.length This removes the length-based timing differential while still rejecting mismatched lengths. Update the comment to match the actual behavior.

The agreement

Both frontier models flagged this within the same line range. AntFleet's unanimous gate fired — the finding posted on the PR. Closed in a58382a.

The fix

0import { timingSafeEqual } from "node:crypto";
1import type { NextRequest } from "next/server";
2import { NextResponse } from "next/server";
3import { logError, logInfo, logWarn, messageOf } from "@/lib/log";
4import { runDailyOnboarderCheckIns } from "@/lib/onboarder";
5import { runOutgoingPrsPoll } from "@/lib/outgoing-prs";
6import { runSweep } from "@/lib/sweep";
7
8// node:crypto + DB driver are Node-only — lock this off Edge.
9export const runtime = "nodejs";
10
11// Sweeper iterates every open finding sequentially against the GitHub
12// API — historically fast enough under the 60s Hobby ceiling. Onboarder
13// check-ins run on the same tick (slice O5); each candidate adds one
14// Anthropic call (~30s) plus a GitHub post. Bumped to 180s (Pro plan
15// has 300s headroom) so a single-digit-partner Phase 2 stays comfortable.
16export const maxDuration = 180;
17
18export async function GET(req: NextRequest): Promise<NextResponse> {
19 const secret = process.env["CRON_SECRET"];
20 if (secret === undefined || secret.length === 0) {
21 logError("cron.misconfigured", { reason: "CRON_SECRET missing" });
22 return new NextResponse("server misconfigured", { status: 500 });
23 }
24 const authHeader = req.headers.get("authorization");
25 // Vercel's cron invocation sets Authorization: Bearer <CRON_SECRET>.
26 // Constant-time compare to deny a length / prefix oracle to anything that
27 // can reach this route before Vercel's edge rate-limit (defense in depth).
28 const expected = `Bearer ${secret}`;
29 const provided = authHeader ?? "";
30 const a = Buffer.from(provided);
31 const b = Buffer.from(expected);
32 if (a.length !== b.length || !timingSafeEqual(a, b)) {
33 logWarn("cron.unauthorized", { hasAuth: authHeader !== null });
34 return new NextResponse("unauthorized", { status: 401 });
35 }
36
37 const t0 = Date.now();
38 try {
39 const result = await runSweep();
40 const sweepMs = Date.now() - t0;
41 logInfo("cron.sweep_complete", {
42 swept: result.swept,
43 closed: result.closed,
44 reactionsRecorded: result.reactionsRecorded,
45 reviewsSkipped: result.reviewsSkipped,
46 errorCount: result.errors.length,
47 elapsedMs: sweepMs,
48 });
49
50 // Onboarder daily check-in runs on the same cron tick as Sweeper.
51 // Self-gates on ONBOARDER_ENABLED so prod stays silent until flipped.
52 // Failure here is logged but does not 5xx the cron — Sweeper success
53 // is the primary load-bearing outcome of this tick.
54 const tOnboarder = Date.now();
55 let onboarderResult: Awaited<ReturnType<typeof runDailyOnboarderCheckIns>> = {
56 attempted: 0,
57 posted: 0,
58 skipped: 0,
59 errors: 0,
60 };
61 try {
62 onboarderResult = await runDailyOnboarderCheckIns(new Date());
63 logInfo("cron.onboarder_checkins_complete", {
64 ...onboarderResult,
65 elapsedMs: Date.now() - tOnboarder,
66 });
67 } catch (err) {
68 const message = messageOf(err);
69 logError("cron.onboarder_checkins_failed", { message });
70 }
71
72 // Cross-repo receipts — poll the merge state of outgoing PRs that
73 // antfleet-ops opened on third-party repos. Wraps its own try/catch
74 // internally (runOutgoingPrsPoll returns null on failure) so it can
75 // never destabilize the cron tick. Skipped silently in environments
76 // without ANTFLEET_OPS_GH_TOKEN — the lazy token read inside
77 // realPollDeps throws and is converted to a log line.
78 const tOutgoing = Date.now();
79 const outgoingResult = await runOutgoingPrsPoll();
80 if (outgoingResult !== null) {
81 logInfo("cron.outgoing_prs_poll_complete", {
82 ...outgoingResult,
83 elapsedMs: Date.now() - tOutgoing,
84 });
85 }
86
87 const elapsedMs = Date.now() - t0;
88 return NextResponse.json({
89 ...result,
90 onboarder: onboarderResult,
91 outgoingPrs: outgoingResult,
92 elapsedMs,
93 });
94 } catch (err) {
95 const message = messageOf(err);
96 logError("cron.sweep_failed", { message });
97 return new NextResponse("sweep failed", { status: 500 });
98 }
99}
100

Closure

Closed 1 week ago

SHA: a58382a1c8934544d327ad62fd4c9c54b187d8ef

View closure receipt on GitHub →

Tweet thread template

tweet 1 of 8157 / 280

Two frontier models reviewed PR #10 on e24ef98c. Both found this bug: low security: Authorization timing check leaks length oracle; comment claims otherwise

tweet 2 of 8126 / 280

The vulnerable code (apps/web/app/api/cron/sweep/route.ts:0-0): (full snippet at https://www.antfleet.dev/anatomy/be39e8a7-2)

tweet 3 of 836 / 280

What Opus saw: "Output unavailable"

tweet 4 of 8280 / 280

What GPT-5 saw: "The code returns early when lengths differ, which creates a measurable timing difference between length-mismatch and equal-length attempts. The nearby comment states the goal is to deny a length/prefix oracle; the current implementation still reveals the expect…

tweet 5 of 897 / 280

Both flagged the same line range. AntFleet's unanimous gate fired — the finding posted on the PR.

tweet 6 of 893 / 280

The fix landed in commit a58382a: (view diff at https://www.antfleet.dev/anatomy/be39e8a7-2)

tweet 7 of 881 / 280

AntFleet reviews every PR with two frontier models. Only unanimous findings post.

tweet 8 of 877 / 280

Full anatomy + reasoning + diffs: https://www.antfleet.dev/anatomy/be39e8a7-2

Paste into X composer one tweet at a time. X has no multi-tweet intent API.