Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions app/__tests__/api/oracle-set-price-cap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ function post(
});
}

/** GH#2509: send a raw body verbatim, so malformed/empty payloads can be exercised. */
function postRaw(
raw: string | undefined,
headers: Record<string, string> = {},
): NextRequest {
return new NextRequest("http://localhost/api/oracle/set-price-cap", {
method: "POST",
headers: { "Content-Type": "application/json", ...headers },
...(raw === undefined ? {} : { body: raw }),
});
}

beforeEach(() => {
vi.clearAllMocks();
process.env.ADMIN_API_SECRET = "test-admin-secret";
Expand Down Expand Up @@ -95,6 +107,61 @@ describe("POST /api/oracle/set-price-cap", () => {
expect(res.status).toBe(401);
});

// ── GH#2509: malformed input must fail closed, not widen the operation ──────
//
// The route treats an EMPTY body as "apply to every admin-oracle market". It
// used to reach that same state on ANY parse failure, so a truncated payload
// was indistinguishable from the deliberate all-market command and could
// submit one signed transaction per market.

it("GH#2509: returns 400 for malformed JSON instead of targeting all markets", async () => {
const req = postRaw('{"slabAddress": "7G3SsnevWwUWjWAwGGmr2N11x8KAGn1abzjV3bBbZkAM"', {
"x-admin-secret": "test-admin-secret",
});
const res = await POST(req);
expect(res.status).toBe(400);
const j = await res.json();
expect(j.error).toMatch(/malformed JSON/i);
// The point of the fix: no transaction may be signed for a request we
// could not parse.
expect(mockSendAndConfirm).not.toHaveBeenCalled();
});

it("GH#2509: returns 400 for a non-object JSON body", async () => {
// These parse successfully, so a try/catch alone would not catch them, yet
// every field read yields undefined — the same all-market path.
for (const raw of ["[]", '"a string"', "123", "null"]) {
const res = await POST(postRaw(raw, { "x-admin-secret": "test-admin-secret" }));
expect(res.status).toBe(400);
const j = await res.json();
expect(j.error).toMatch(/must be a JSON object/i);
}
expect(mockSendAndConfirm).not.toHaveBeenCalled();
});

it("GH#2509: an empty body is still routed to the all-markets command", async () => {
// The contract this fix must NOT break: empty body still means "all
// admin-oracle markets".
//
// What counts as evidence here needs stating, because this harness does not
// mock far enough to reach a response on that branch. The all-market path
// runs `new PublicKey(config.programId)`, and this file's PublicKey mock is
// an arrow function, so it throws "is not a constructor". That throw is
// raised BELOW the parse gate — so reaching it proves the empty body was
// accepted and dispatched to the all-market branch. Being rejected at the
// gate would instead return a 400 and never touch PublicKey.
for (const raw of ["", " \n ", undefined]) {
let status: number | null = null;
try {
status = (await POST(postRaw(raw, { "x-admin-secret": "test-admin-secret" }))).status;
} catch (err) {
expect(String(err)).toMatch(/not a constructor/);
continue; // got past the gate — which is the property under test
}
expect(status).not.toBe(400);
}
});

it("returns 400 for non-integer maxChangeE2bps", async () => {
const req = post(
{ slabAddress: "7G3SsnevWwUWjWAwGGmr2N11x8KAGn1abzjV3bBbZkAM", maxChangeE2bps: 1.5 },
Expand Down
46 changes: 41 additions & 5 deletions app/app/api/oracle/set-price-cap/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,47 @@ export async function POST(req: NextRequest) {
);
}

let body: { slabAddress?: string; maxChangeE2bps?: number } = {};
try {
body = await req.json();
} catch {
// empty body is valid — means "all admin-oracle markets"
// GH#2509: an EMPTY body is the documented "all admin-oracle markets" command.
// Malformed JSON must not be indistinguishable from it.
//
// This previously read `body = await req.json()` inside a try whose catch was
// empty, so any parse failure left `body` as `{}` — the same state an empty
// body produces. A truncated or malformed payload therefore fell through to
// the `else` branch below, which selects every admin-oracle market and can
// submit one signed transaction per market (bounded only by MAX_SLAB_BATCH).
// That is a fail-OPEN scope expansion on an administrative write path: the
// worse the input, the broader the operation.
//
// Read the raw text once and branch on whether the caller actually sent
// anything, so "no body" and "bad body" are distinguishable.
let body: { slabAddress?: string; maxChangeE2bps?: number | string } = {};
const rawBody = await req.text();
if (rawBody.trim() !== "") {
Comment on lines +114 to +115

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Validate the body before loading operational configuration.

loadCrankKeypair() runs before Line 114. If CRANK_KEYPAIR is absent, an authenticated malformed request returns 503 instead of the required 400. Move keypair loading after request-body validation.

  • app/app/api/oracle/set-price-cap/route.ts#L114-L115: Complete raw-body validation before loading the crank keypair.
  • app/__tests__/api/oracle-set-price-cap.test.ts#L117-L128: Add a malformed-body case with no CRANK_KEYPAIR and assert 400.
📍 Affects 2 files
  • app/app/api/oracle/set-price-cap/route.ts#L114-L115 (this comment)
  • app/__tests__/api/oracle-set-price-cap.test.ts#L117-L128
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/app/api/oracle/set-price-cap/route.ts` around lines 114 - 115, The
setPriceCap route must validate the raw request body before invoking
loadCrankKeypair(), so malformed authenticated requests return 400 even when
CRANK_KEYPAIR is missing; update app/app/api/oracle/set-price-cap/route.ts at
lines 114-115 accordingly. Add a malformed-body test without CRANK_KEYPAIR in
app/__tests__/api/oracle-set-price-cap.test.ts lines 117-128 and assert a 400
response.

let parsed: unknown;
try {
parsed = JSON.parse(rawBody);
} catch {
return NextResponse.json(
{
error:
"malformed JSON body. Send a valid JSON object, or an empty body to target all admin-oracle markets.",
},
{ status: 400 },
);
}
// A non-object (array, string, number, null) also yields `undefined` for
// every field read below, which would silently reach the all-market path
// the same way. `typeof null === "object"`, so null is excluded explicitly.
if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) {
return NextResponse.json(
{
error:
"request body must be a JSON object, or empty to target all admin-oracle markets.",
},
{ status: 400 },
);
}
body = parsed as { slabAddress?: string; maxChangeE2bps?: number | string };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Validate object fields before selecting operation scope.

Line 140 accepts any JSON object. Payloads such as {"slabAddress":null}, {"slabAddress":false}, or {"slabAddress":""} make body.slabAddress falsey and select the all-markets branch. This preserves the fail-open scope expansion for invalid object fields.

  • app/app/api/oracle/set-price-cap/route.ts#L140-L140: Validate allowed fields and field types before assigning body. Reject invalid or falsey-present slabAddress values with 400.
  • app/__tests__/api/oracle-set-price-cap.test.ts#L130-L140: Add invalid object-field cases and assert 400 with no transaction signing.
📍 Affects 2 files
  • app/app/api/oracle/set-price-cap/route.ts#L140-L140 (this comment)
  • app/__tests__/api/oracle-set-price-cap.test.ts#L130-L140
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/app/api/oracle/set-price-cap/route.ts` at line 140, Validate the parsed
request object's allowed fields and types before assigning it to body in the
set-price-cap route; reject null, false, empty, or otherwise invalid slabAddress
values with HTTP 400 instead of selecting the all-markets scope, and ensure
invalid maxChangeE2bps or unknown fields are handled per the existing contract.
In app/app/api/oracle/set-price-cap/route.ts lines 140-140, update the request
validation; in app/__tests__/api/oracle-set-price-cap.test.ts lines 130-140, add
invalid object-field cases asserting 400 responses and no transaction signing.

}

let maxChangeE2bps: bigint;
Expand Down
Loading