mirror of
https://github.com/farcasclaudiu/openclaw.git
synced 2026-06-28 21:01:43 +03:00
fix: validate state for manual Chutes OAuth
This commit is contained in:
+1
-1
@@ -32,7 +32,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- Telegram: when `channels.telegram.commands.native` is `false`, exclude plugin commands from `setMyCommands` menu registration while keeping plugin slash handlers callable. (#15132) Thanks @Glucksberg.
|
- Telegram: when `channels.telegram.commands.native` is `false`, exclude plugin commands from `setMyCommands` menu registration while keeping plugin slash handlers callable. (#15132) Thanks @Glucksberg.
|
||||||
- Security/Agents: scope CLI process cleanup to owned child PIDs to avoid killing unrelated processes on shared hosts. Thanks @aether-ai-agent.
|
- Security/Agents: scope CLI process cleanup to owned child PIDs to avoid killing unrelated processes on shared hosts. Thanks @aether-ai-agent.
|
||||||
- Security/Agents (macOS): prevent shell injection when writing Claude CLI keychain credentials. (#15924) Thanks @aether-ai-agent.
|
- Security/Agents (macOS): prevent shell injection when writing Claude CLI keychain credentials. (#15924) Thanks @aether-ai-agent.
|
||||||
- Security: fix Chutes manual OAuth login state validation (thanks @aether-ai-agent). (#16058)
|
- Security: fix Chutes manual OAuth login state validation by requiring the full redirect URL (reject code-only pastes) (thanks @aether-ai-agent).
|
||||||
- Security/Tlon: harden Urbit URL fetching against SSRF by blocking private/internal hosts by default (opt-in: `channels.tlon.allowPrivateNetwork`). Thanks @p80n-sec.
|
- Security/Tlon: harden Urbit URL fetching against SSRF by blocking private/internal hosts by default (opt-in: `channels.tlon.allowPrivateNetwork`). Thanks @p80n-sec.
|
||||||
- Security/Voice Call (Telnyx): require webhook signature verification when receiving inbound events; configs without `telnyx.publicKey` are now rejected unless `skipSignatureVerification` is enabled. Thanks @p80n-sec.
|
- Security/Voice Call (Telnyx): require webhook signature verification when receiving inbound events; configs without `telnyx.publicKey` are now rejected unless `skipSignatureVerification` is enabled. Thanks @p80n-sec.
|
||||||
- Security/Discovery: stop treating Bonjour TXT records as authoritative routing (prefer resolved service endpoints) and prevent discovery from overriding stored TLS pins; autoconnect now requires a previously trusted gateway. Thanks @simecek.
|
- Security/Discovery: stop treating Bonjour TXT records as authoritative routing (prefer resolved service endpoints) and prevent discovery from overriding stored TLS pins; autoconnect now requires a previously trusted gateway. Thanks @simecek.
|
||||||
|
|||||||
@@ -2,53 +2,51 @@ import { describe, expect, it } from "vitest";
|
|||||||
import { generateChutesPkce, parseOAuthCallbackInput } from "./chutes-oauth.js";
|
import { generateChutesPkce, parseOAuthCallbackInput } from "./chutes-oauth.js";
|
||||||
|
|
||||||
describe("parseOAuthCallbackInput", () => {
|
describe("parseOAuthCallbackInput", () => {
|
||||||
const EXPECTED_STATE = "abc123def456";
|
it("rejects code-only input (state required)", () => {
|
||||||
|
const parsed = parseOAuthCallbackInput("abc123", "expected-state");
|
||||||
|
expect(parsed).toEqual({
|
||||||
|
error: "Paste the full redirect URL (must include code + state).",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("returns code and state for valid URL with matching state", () => {
|
it("accepts full redirect URL when state matches", () => {
|
||||||
const result = parseOAuthCallbackInput(
|
const parsed = parseOAuthCallbackInput(
|
||||||
`http://localhost/cb?code=authcode_xyz&state=${EXPECTED_STATE}`,
|
"http://127.0.0.1:1456/oauth-callback?code=abc123&state=expected-state",
|
||||||
EXPECTED_STATE,
|
"expected-state",
|
||||||
);
|
);
|
||||||
expect(result).toEqual({ code: "authcode_xyz", state: EXPECTED_STATE });
|
expect(parsed).toEqual({ code: "abc123", state: "expected-state" });
|
||||||
});
|
});
|
||||||
|
|
||||||
it("rejects URL with mismatched state (CSRF protection)", () => {
|
it("accepts querystring-only input when state matches", () => {
|
||||||
const result = parseOAuthCallbackInput(
|
const parsed = parseOAuthCallbackInput("code=abc123&state=expected-state", "expected-state");
|
||||||
"http://localhost/cb?code=authcode_xyz&state=attacker_state",
|
expect(parsed).toEqual({ code: "abc123", state: "expected-state" });
|
||||||
EXPECTED_STATE,
|
});
|
||||||
|
|
||||||
|
it("rejects missing state", () => {
|
||||||
|
const parsed = parseOAuthCallbackInput(
|
||||||
|
"http://127.0.0.1:1456/oauth-callback?code=abc123",
|
||||||
|
"expected-state",
|
||||||
);
|
);
|
||||||
expect(result).toHaveProperty("error");
|
expect(parsed).toEqual({
|
||||||
expect((result as { error: string }).error).toMatch(/state mismatch/i);
|
error: "Missing 'state' parameter. Paste the full redirect URL.",
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it("accepts bare code input for manual flow", () => {
|
it("rejects state mismatch", () => {
|
||||||
const result = parseOAuthCallbackInput("bare_auth_code", EXPECTED_STATE);
|
const parsed = parseOAuthCallbackInput(
|
||||||
expect(result).toEqual({ code: "bare_auth_code", state: EXPECTED_STATE });
|
"http://127.0.0.1:1456/oauth-callback?code=abc123&state=evil",
|
||||||
});
|
"expected-state",
|
||||||
|
|
||||||
it("rejects empty input", () => {
|
|
||||||
const result = parseOAuthCallbackInput("", EXPECTED_STATE);
|
|
||||||
expect(result).toEqual({ error: "No input provided" });
|
|
||||||
});
|
|
||||||
|
|
||||||
it("rejects URL missing code parameter", () => {
|
|
||||||
const result = parseOAuthCallbackInput(
|
|
||||||
`http://localhost/cb?state=${EXPECTED_STATE}`,
|
|
||||||
EXPECTED_STATE,
|
|
||||||
);
|
);
|
||||||
expect(result).toHaveProperty("error");
|
expect(parsed).toEqual({
|
||||||
});
|
error: "OAuth state mismatch - possible CSRF attack. Please retry login.",
|
||||||
|
});
|
||||||
it("rejects URL missing state parameter", () => {
|
|
||||||
const result = parseOAuthCallbackInput("http://localhost/cb?code=authcode_xyz", EXPECTED_STATE);
|
|
||||||
expect(result).toHaveProperty("error");
|
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("generateChutesPkce", () => {
|
describe("generateChutesPkce", () => {
|
||||||
it("returns verifier and challenge strings", () => {
|
it("returns verifier and challenge", () => {
|
||||||
const pkce = generateChutesPkce();
|
const pkce = generateChutesPkce();
|
||||||
expect(pkce.verifier).toMatch(/^[0-9a-f]{64}$/);
|
expect(pkce.verifier).toMatch(/^[0-9a-f]{64}$/);
|
||||||
expect(pkce.challenge).toBeTruthy();
|
expect(pkce.challenge).toMatch(/^[A-Za-z0-9_-]+$/);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
+32
-18
@@ -42,28 +42,42 @@ export function parseOAuthCallbackInput(
|
|||||||
return { error: "No input provided" };
|
return { error: "No input provided" };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Manual flow must validate CSRF state; require URL (or querystring) that includes `state`.
|
||||||
|
let url: URL;
|
||||||
try {
|
try {
|
||||||
const url = new URL(trimmed);
|
url = new URL(trimmed);
|
||||||
const code = url.searchParams.get("code");
|
|
||||||
const state = url.searchParams.get("state");
|
|
||||||
if (!code) {
|
|
||||||
return { error: "Missing 'code' parameter in URL" };
|
|
||||||
}
|
|
||||||
if (!state) {
|
|
||||||
return { error: "Missing 'state' parameter. Paste the full URL (or just the code)." };
|
|
||||||
}
|
|
||||||
if (state !== expectedState) {
|
|
||||||
return { error: "OAuth state mismatch - possible CSRF attack. Please retry login." };
|
|
||||||
}
|
|
||||||
return { code, state };
|
|
||||||
} catch {
|
} catch {
|
||||||
// Manual flow: users often paste only the authorization code.
|
// Code-only paste (common) is no longer accepted because it defeats state validation.
|
||||||
// In that case we can't validate state, but the user is explicitly opting in by pasting it.
|
if (
|
||||||
if (!/\s/.test(trimmed) && !trimmed.includes("://") && trimmed.length > 0) {
|
!/\s/.test(trimmed) &&
|
||||||
return { code: trimmed, state: expectedState };
|
!trimmed.includes("://") &&
|
||||||
|
!trimmed.includes("?") &&
|
||||||
|
!trimmed.includes("=")
|
||||||
|
) {
|
||||||
|
return { error: "Paste the full redirect URL (must include code + state)." };
|
||||||
|
}
|
||||||
|
|
||||||
|
// Users sometimes paste only the query string: `?code=...&state=...` or `code=...&state=...`
|
||||||
|
const qs = trimmed.startsWith("?") ? trimmed : `?${trimmed}`;
|
||||||
|
try {
|
||||||
|
url = new URL(`http://localhost/${qs}`);
|
||||||
|
} catch {
|
||||||
|
return { error: "Paste the full redirect URL (must include code + state)." };
|
||||||
}
|
}
|
||||||
return { error: "Paste the redirect URL (or authorization code)." };
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const code = url.searchParams.get("code")?.trim();
|
||||||
|
const state = url.searchParams.get("state")?.trim();
|
||||||
|
if (!code) {
|
||||||
|
return { error: "Missing 'code' parameter in URL" };
|
||||||
|
}
|
||||||
|
if (!state) {
|
||||||
|
return { error: "Missing 'state' parameter. Paste the full redirect URL." };
|
||||||
|
}
|
||||||
|
if (state !== expectedState) {
|
||||||
|
return { error: "OAuth state mismatch - possible CSRF attack. Please retry login." };
|
||||||
|
}
|
||||||
|
return { code, state };
|
||||||
}
|
}
|
||||||
|
|
||||||
function coerceExpiresAt(expiresInSeconds: number, now: number): number {
|
function coerceExpiresAt(expiresInSeconds: number, now: number): number {
|
||||||
|
|||||||
@@ -1127,7 +1127,7 @@ describe("applyAuthChoice", () => {
|
|||||||
|
|
||||||
expect(text).toHaveBeenCalledWith(
|
expect(text).toHaveBeenCalledWith(
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
message: "Paste the redirect URL (or authorization code)",
|
message: "Paste the redirect URL",
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
expect(result.config.auth?.profiles?.["chutes:remote-user"]).toMatchObject({
|
expect(result.config.auth?.profiles?.["chutes:remote-user"]).toMatchObject({
|
||||||
|
|||||||
@@ -156,7 +156,7 @@ export async function loginChutes(params: {
|
|||||||
await params.onAuth({ url });
|
await params.onAuth({ url });
|
||||||
params.onProgress?.("Waiting for redirect URL…");
|
params.onProgress?.("Waiting for redirect URL…");
|
||||||
const input = await params.onPrompt({
|
const input = await params.onPrompt({
|
||||||
message: "Paste the redirect URL (or authorization code)",
|
message: "Paste the redirect URL",
|
||||||
placeholder: `${params.app.redirectUri}?code=...&state=...`,
|
placeholder: `${params.app.redirectUri}?code=...&state=...`,
|
||||||
});
|
});
|
||||||
const parsed = parseOAuthCallbackInput(String(input), state);
|
const parsed = parseOAuthCallbackInput(String(input), state);
|
||||||
@@ -176,7 +176,7 @@ export async function loginChutes(params: {
|
|||||||
}).catch(async () => {
|
}).catch(async () => {
|
||||||
params.onProgress?.("OAuth callback not detected; paste redirect URL…");
|
params.onProgress?.("OAuth callback not detected; paste redirect URL…");
|
||||||
const input = await params.onPrompt({
|
const input = await params.onPrompt({
|
||||||
message: "Paste the redirect URL (or authorization code)",
|
message: "Paste the redirect URL",
|
||||||
placeholder: `${params.app.redirectUri}?code=...&state=...`,
|
placeholder: `${params.app.redirectUri}?code=...&state=...`,
|
||||||
});
|
});
|
||||||
const parsed = parseOAuthCallbackInput(String(input), state);
|
const parsed = parseOAuthCallbackInput(String(input), state);
|
||||||
|
|||||||
@@ -17,8 +17,7 @@ export function createVpsAwareOAuthHandlers(params: {
|
|||||||
onAuth: (event: { url: string }) => Promise<void>;
|
onAuth: (event: { url: string }) => Promise<void>;
|
||||||
onPrompt: (prompt: OAuthPrompt) => Promise<string>;
|
onPrompt: (prompt: OAuthPrompt) => Promise<string>;
|
||||||
} {
|
} {
|
||||||
const manualPromptMessage =
|
const manualPromptMessage = params.manualPromptMessage ?? "Paste the redirect URL";
|
||||||
params.manualPromptMessage ?? "Paste the redirect URL (or authorization code)";
|
|
||||||
let manualCodePromise: Promise<string> | undefined;
|
let manualCodePromise: Promise<string> | undefined;
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|||||||
Reference in New Issue
Block a user