From f3c10e4302fc441b184f2bcb92f68889d4c28607 Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Sun, 18 Oct 2026 18:15:36 +0100 Subject: [PATCH 1/2] fix: harden well-known auth login and address CodeQL alerts - auth login : require https (http only for loopback), validate the .well-known/aictrl auth document, and confirm before running the server-provided command (CodeQL #16, #17) - ci: restrict workflow GITHUB_TOKEN to contents: read (CodeQL #14) - codex: generate the PKCE verifier from 32 unbiased random bytes per RFC 7636 (CodeQL #5) Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 3 +++ packages/cli/src/auth/wellknown.ts | 31 ++++++++++++++++++++++++ packages/cli/src/cli/cmd/auth.ts | 17 ++++++++++--- packages/cli/src/plugin/codex.ts | 11 ++------- packages/cli/test/auth/wellknown.test.ts | 30 +++++++++++++++++++++++ 5 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 packages/cli/src/auth/wellknown.ts create mode 100644 packages/cli/test/auth/wellknown.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 65dcd96a..435b79d3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,9 @@ on: pull_request: branches: [main] +permissions: + contents: read + jobs: verify: runs-on: ubuntu-latest diff --git a/packages/cli/src/auth/wellknown.ts b/packages/cli/src/auth/wellknown.ts new file mode 100644 index 00000000..7cd15178 --- /dev/null +++ b/packages/cli/src/auth/wellknown.ts @@ -0,0 +1,31 @@ +import z from "zod" + +export namespace WellKnownAuth { + const LOOPBACK_HOSTS = new Set(["localhost", "127.0.0.1", "[::1]"]) + + export const Document = z.object({ + auth: z.object({ + command: z.array(z.string().min(1)).min(1), + env: z.string().min(1), + }), + }) + export type Command = z.infer["auth"] + + // The provider dictates a local command, so plain http is only allowed for local development. + export function endpoint(providerURL: string): string { + const url = new URL(providerURL) + const loopback = url.protocol === "http:" && LOOPBACK_HOSTS.has(url.hostname) + if (url.protocol !== "https:" && !loopback) { + throw new Error(`Auth provider URL must use https: ${providerURL}`) + } + return `${providerURL}/.well-known/aictrl` + } + + export async function fetchCommand(providerURL: string): Promise { + const response = await fetch(endpoint(providerURL)) + if (!response.ok) { + throw new Error(`Failed to fetch auth provider config from ${providerURL}: ${response.status}`) + } + return Document.parse(await response.json()).auth + } +} diff --git a/packages/cli/src/cli/cmd/auth.ts b/packages/cli/src/cli/cmd/auth.ts index b4e6108d..3839ffe9 100644 --- a/packages/cli/src/cli/cmd/auth.ts +++ b/packages/cli/src/cli/cmd/auth.ts @@ -1,4 +1,5 @@ import { Auth } from "../../auth" +import { WellKnownAuth } from "../../auth/wellknown" import { cmd } from "./cmd" import * as prompts from "@clack/prompts" import { UI } from "../ui" @@ -263,9 +264,17 @@ export const AuthLoginCommand = cmd({ UI.empty() prompts.intro("Add credential") if (args.url) { - const wellknown = await fetch(`${args.url}/.well-known/aictrl`).then((x) => x.json() as any) - prompts.log.info(`Running \`${wellknown.auth.command.join(" ")}\``) - const proc = Process.spawn(wellknown.auth.command, { + const wellknown = await WellKnownAuth.fetchCommand(args.url) + const approved = await prompts.confirm({ + message: `${args.url} wants to run \`${wellknown.command.join(" ")}\` to get a token. Run it?`, + initialValue: false, + }) + if (prompts.isCancel(approved)) throw new UI.CancelledError() + if (!approved) { + prompts.outro("Cancelled") + return + } + const proc = Process.spawn(wellknown.command, { stdout: "pipe", }) if (!proc.stdout) { @@ -281,7 +290,7 @@ export const AuthLoginCommand = cmd({ } await Auth.set(args.url, { type: "wellknown", - key: wellknown.auth.env, + key: wellknown.env, token: token.trim(), }) prompts.log.success("Logged into " + args.url) diff --git a/packages/cli/src/plugin/codex.ts b/packages/cli/src/plugin/codex.ts index 3552377c..27abe438 100644 --- a/packages/cli/src/plugin/codex.ts +++ b/packages/cli/src/plugin/codex.ts @@ -20,7 +20,8 @@ interface PkceCodes { } async function generatePKCE(): Promise { - const verifier = generateRandomString(43) + // RFC 7636: base64url-encode 32 random bytes to get an unbiased 43-character verifier. + const verifier = base64UrlEncode(crypto.getRandomValues(new Uint8Array(32)).buffer) const encoder = new TextEncoder() const data = encoder.encode(verifier) const hash = await crypto.subtle.digest("SHA-256", data) @@ -28,14 +29,6 @@ async function generatePKCE(): Promise { return { verifier, challenge } } -function generateRandomString(length: number): string { - const chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~" - const bytes = crypto.getRandomValues(new Uint8Array(length)) - return Array.from(bytes) - .map((b) => chars[b % chars.length]) - .join("") -} - function base64UrlEncode(buffer: ArrayBuffer): string { const bytes = new Uint8Array(buffer) const binary = String.fromCharCode(...bytes) diff --git a/packages/cli/test/auth/wellknown.test.ts b/packages/cli/test/auth/wellknown.test.ts new file mode 100644 index 00000000..1e8eac1b --- /dev/null +++ b/packages/cli/test/auth/wellknown.test.ts @@ -0,0 +1,30 @@ +import { test, expect } from "bun:test" +import { WellKnownAuth } from "../../src/auth/wellknown" + +test("builds the well-known endpoint for https providers", () => { + expect(WellKnownAuth.endpoint("https://auth.example.com")).toBe("https://auth.example.com/.well-known/aictrl") +}) + +test("allows plain http only for loopback hosts", () => { + expect(WellKnownAuth.endpoint("http://localhost:4096")).toBe("http://localhost:4096/.well-known/aictrl") + expect(WellKnownAuth.endpoint("http://127.0.0.1:4096")).toBe("http://127.0.0.1:4096/.well-known/aictrl") + expect(WellKnownAuth.endpoint("http://[::1]:4096")).toBe("http://[::1]:4096/.well-known/aictrl") +}) + +test("rejects insecure or non-http provider URLs", () => { + expect(() => WellKnownAuth.endpoint("http://auth.example.com")).toThrow("must use https") + expect(() => WellKnownAuth.endpoint("file:///etc/passwd")).toThrow("must use https") + expect(() => WellKnownAuth.endpoint("auth.example.com")).toThrow() +}) + +test("accepts a well-formed auth command", () => { + const parsed = WellKnownAuth.Document.parse({ auth: { command: ["cli", "token"], env: "TOKEN" }, config: {} }) + expect(parsed.auth).toEqual({ command: ["cli", "token"], env: "TOKEN" }) +}) + +test("rejects malformed auth commands", () => { + expect(() => WellKnownAuth.Document.parse({ auth: { command: "cli token", env: "TOKEN" } })).toThrow() + expect(() => WellKnownAuth.Document.parse({ auth: { command: [], env: "TOKEN" } })).toThrow() + expect(() => WellKnownAuth.Document.parse({ auth: { command: ["cli"] } })).toThrow() + expect(() => WellKnownAuth.Document.parse({})).toThrow() +}) From 4cc5b9c93f9bb391a3cafcbd5ed2660fe2908597 Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Wed, 21 Oct 2026 11:55:27 +0100 Subject: [PATCH 2/2] fix(auth): address well-known login review findings --- packages/cli/src/auth/wellknown.ts | 57 +++++++++++-- packages/cli/src/cli/cmd/auth.ts | 4 +- packages/cli/test/auth/wellknown.test.ts | 101 +++++++++++++++++++++++ 3 files changed, 154 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/auth/wellknown.ts b/packages/cli/src/auth/wellknown.ts index 7cd15178..ad1bd852 100644 --- a/packages/cli/src/auth/wellknown.ts +++ b/packages/cli/src/auth/wellknown.ts @@ -13,19 +13,64 @@ export namespace WellKnownAuth { // The provider dictates a local command, so plain http is only allowed for local development. export function endpoint(providerURL: string): string { - const url = new URL(providerURL) + let url: URL + try { + url = new URL(providerURL) + } catch (cause) { + throw new Error("Invalid auth provider URL. Supply an absolute https URL.", { cause }) + } const loopback = url.protocol === "http:" && LOOPBACK_HOSTS.has(url.hostname) if (url.protocol !== "https:" && !loopback) { - throw new Error(`Auth provider URL must use https: ${providerURL}`) + throw new Error("Auth provider URL must use https (except http on loopback hosts).") } - return `${providerURL}/.well-known/aictrl` + url.search = "" + url.hash = "" + url.pathname = `${url.pathname.replace(/\/+$/, "")}/.well-known/aictrl` + return url.toString() + } + + export function display(value: string | readonly string[]): string { + return JSON.stringify(value).replace( + /[\u007f-\u009f\u061c\u200e\u200f\u2028-\u202e\u2066-\u2069]/g, + (character) => `\\u${character.charCodeAt(0).toString(16).padStart(4, "0")}`, + ) } export async function fetchCommand(providerURL: string): Promise { - const response = await fetch(endpoint(providerURL)) + const target = endpoint(providerURL) + let response: Response + try { + response = await fetch(target, { redirect: "error" }) + } catch (cause) { + throw new Error( + "Failed to fetch auth provider config. Check connectivity and use a direct URL without redirects.", + { + cause, + }, + ) + } if (!response.ok) { - throw new Error(`Failed to fetch auth provider config from ${providerURL}: ${response.status}`) + throw new Error(`Failed to fetch auth provider config (HTTP ${response.status}). Check the provider URL.`, { + cause: response, + }) + } + let document: unknown + try { + document = await response.json() + } catch (cause) { + throw new Error("Auth provider config is not valid JSON. Contact the provider to fix its well-known document.", { + cause, + }) + } + try { + return Document.parse(document).auth + } catch (cause) { + throw new Error( + "Auth provider config must supply auth.command as a nonempty string array and auth.env as a string.", + { + cause, + }, + ) } - return Document.parse(await response.json()).auth } } diff --git a/packages/cli/src/cli/cmd/auth.ts b/packages/cli/src/cli/cmd/auth.ts index 3839ffe9..fcf36a66 100644 --- a/packages/cli/src/cli/cmd/auth.ts +++ b/packages/cli/src/cli/cmd/auth.ts @@ -266,7 +266,7 @@ export const AuthLoginCommand = cmd({ if (args.url) { const wellknown = await WellKnownAuth.fetchCommand(args.url) const approved = await prompts.confirm({ - message: `${args.url} wants to run \`${wellknown.command.join(" ")}\` to get a token. Run it?`, + message: `${WellKnownAuth.display(args.url)} wants to run ${WellKnownAuth.display(wellknown.command)} to get a token. Run it?`, initialValue: false, }) if (prompts.isCancel(approved)) throw new UI.CancelledError() @@ -293,7 +293,7 @@ export const AuthLoginCommand = cmd({ key: wellknown.env, token: token.trim(), }) - prompts.log.success("Logged into " + args.url) + prompts.log.success("Logged into " + WellKnownAuth.display(args.url)) prompts.outro("Done") return } diff --git a/packages/cli/test/auth/wellknown.test.ts b/packages/cli/test/auth/wellknown.test.ts index 1e8eac1b..ffa2529f 100644 --- a/packages/cli/test/auth/wellknown.test.ts +++ b/packages/cli/test/auth/wellknown.test.ts @@ -28,3 +28,104 @@ test("rejects malformed auth commands", () => { expect(() => WellKnownAuth.Document.parse({ auth: { command: ["cli"] } })).toThrow() expect(() => WellKnownAuth.Document.parse({})).toThrow() }) + +test("appends the endpoint to the base path without query, fragment, or trailing slashes", () => { + for (const suffix of ["", "/", "///", "?tenant=example", "/?tenant=example#login"]) { + expect(WellKnownAuth.endpoint(`https://auth.example.com/base${suffix}`)).toBe( + "https://auth.example.com/base/.well-known/aictrl", + ) + } +}) + +test("malformed provider URLs have a minimal actionable message and retain the cause", () => { + try { + WellKnownAuth.endpoint("invalid-owned-url\nPRIVATE_DOCUMENT_MARKER") + throw new Error("Expected an invalid URL error") + } catch (error) { + expect(error).toBeInstanceOf(Error) + expect((error as Error).message).toContain("Supply an absolute https URL") + expect((error as Error).message).not.toContain("PRIVATE_DOCUMENT_MARKER") + expect((error as Error).cause).toBeInstanceOf(Error) + } +}) + +test("terminal display escapes controls while preserving exact values and argument boundaries", () => { + const command = ["owned-cli", "two words", 'quote" and `literal`', "\x1b[2J\x07\r\n\t\0\x7f\x85\u2028\u2029\u202e"] + const rendered = WellKnownAuth.display(command) + expect(JSON.parse(rendered)).toEqual(command) + expect(rendered).not.toMatch(/[\x00-\x1f\x7f-\x9f\u2028\u2029\u202e]/) + expect(rendered).toContain('"two words"') + expect(rendered).toContain("\\u001b") + expect(rendered).toContain("\\u202e") + const provider = "https://auth.example.com/?hint=\x1b[2J\n\u2066" + expect(JSON.parse(WellKnownAuth.display(provider))).toBe(provider) + expect(WellKnownAuth.display(provider)).not.toMatch(/[\x00-\x1f\u2066]/) +}) + +test("fetches the canonical endpoint and returns only the validated auth command", async () => { + const paths: string[] = [] + const server = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch(request) { + paths.push(new URL(request.url).pathname) + return Response.json({ auth: { command: ["owned-cli", "two words"], env: "OWNED_TOKEN" } }) + }, + }) + try { + expect(await WellKnownAuth.fetchCommand(`${server.url}base///?tenant=example#login`)).toEqual({ + command: ["owned-cli", "two words"], + env: "OWNED_TOKEN", + }) + expect(paths).toEqual(["/base/.well-known/aictrl"]) + } finally { + await server.stop(true) + } +}) + +test("never follows an auth document redirect to another owned loopback server", async () => { + let followed = 0 + const target = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch() { + followed++ + return Response.json({ auth: { command: ["owned-cli"], env: "OWNED_TOKEN" } }) + }, + }) + const redirect = Bun.serve({ + hostname: "127.0.0.1", + port: 0, + fetch: () => Response.redirect(target.url), + }) + try { + await expect(WellKnownAuth.fetchCommand(redirect.url.toString())).rejects.toThrow("without redirects") + expect(followed).toBe(0) + } finally { + await Promise.all([redirect.stop(true), target.stop(true)]) + } +}) + +for (const [name, response, message] of [ + ["HTTP error", () => new Response("PRIVATE_DOCUMENT_MARKER", { status: 503 }), "HTTP 503"], + ["invalid JSON", () => new Response("PRIVATE_DOCUMENT_MARKER"), "not valid JSON"], + ["invalid schema", () => Response.json({ auth: { command: "PRIVATE_DOCUMENT_MARKER" } }), "auth.command"], +] as const) { + test(`${name} produces an actionable error with cause without echoing the document`, async () => { + const server = Bun.serve({ hostname: "127.0.0.1", port: 0, fetch: response }) + try { + let failure: Error | undefined + try { + await WellKnownAuth.fetchCommand(server.url.toString()) + } catch (error) { + failure = error as Error + } + expect(failure).toBeInstanceOf(Error) + expect(failure!.message).toContain(message) + expect(failure!.message).not.toContain("PRIVATE_DOCUMENT_MARKER") + expect(failure!.cause).toBeDefined() + } finally { + await server.stop(true) + } + }) +}