Skip to content

Commit fdade69

Browse files
committed
fix: address PR 3173 review feedback
- inline prototype-pollution guards at JSON Patch assignment sites in chat-client.ts so CodeQL can statically verify them (Set.has() check upstream wasn't being traced) - wrap JSON.parse(payloadStr) in playground action's start handler to return 400 on malformed JSON instead of 500
1 parent 1d06d1d commit fdade69

2 files changed

Lines changed: 20 additions & 13 deletions

File tree

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.playground.action.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,12 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
100100
// for the first run trigger. After session create, the agent
101101
// reads subsequent messages from `.in/append` so the payload
102102
// here is just the bootstrap.
103-
const payload = payloadStr ? (JSON.parse(payloadStr) as Record<string, any>) : {};
103+
let payload: Record<string, any> = {};
104+
try {
105+
payload = payloadStr ? (JSON.parse(payloadStr) as Record<string, any>) : {};
106+
} catch {
107+
return json({ error: "Invalid payload JSON" }, { status: 400 });
108+
}
104109

105110
let parsedClientData: unknown;
106111
try {

packages/core/src/v3/chat-client.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,21 +113,17 @@ function readPointer(doc: unknown, tokens: string[]): unknown {
113113
return cursor;
114114
}
115115

116-
// Defense-in-depth: parseJsonPointer already rejects these segments, but
117-
// CodeQL's prototype-pollution analysis doesn't trace through that boundary.
118-
// The local check at the assignment site keeps the static analysis happy and
119-
// guards against any future caller that bypasses parseJsonPointer.
120-
function assertSafeKey(token: string): void {
121-
if (FORBIDDEN_POINTER_SEGMENTS.has(token)) {
122-
throw new Error(`Refusing to mutate forbidden key "${token}"`);
123-
}
124-
}
125-
126116
function removeAt(parent: any, lastToken: string): void {
127117
if (Array.isArray(parent)) {
128118
parent.splice(Number(lastToken), 1);
129119
} else if (parent && typeof parent === "object") {
130-
assertSafeKey(lastToken);
120+
if (
121+
lastToken === "__proto__" ||
122+
lastToken === "constructor" ||
123+
lastToken === "prototype"
124+
) {
125+
throw new Error(`Refusing to mutate forbidden key "${lastToken}"`);
126+
}
131127
delete parent[lastToken];
132128
} else {
133129
throw new Error("Cannot remove: parent is not a container");
@@ -140,7 +136,13 @@ function insertAt(parent: any, lastToken: string, value: unknown, op: "add" | "r
140136
if (op === "add") parent.splice(idx, 0, value);
141137
else parent[idx] = value;
142138
} else if (parent && typeof parent === "object") {
143-
assertSafeKey(lastToken);
139+
if (
140+
lastToken === "__proto__" ||
141+
lastToken === "constructor" ||
142+
lastToken === "prototype"
143+
) {
144+
throw new Error(`Refusing to mutate forbidden key "${lastToken}"`);
145+
}
144146
parent[lastToken] = value;
145147
} else {
146148
throw new Error("Cannot insert: parent is not a container");

0 commit comments

Comments
 (0)