fix: harden auth response validation

This commit is contained in:
2026-09-03 18:08:43 +09:00
parent bc810beb3b
commit 3a94c845cf
8 changed files with 448 additions and 34 deletions
+104 -19
View File
@@ -90,6 +90,30 @@ pub async fn poll_device_login(
parse_json_response(response).await
}
fn device_login_poll_result(
response: DeviceLoginPollResponse,
) -> Result<Option<String>, BackendAuthClientError> {
match response.status {
DeviceLoginPollStatus::Approved => response
.access_token
.ok_or(BackendAuthClientError::MissingAccessToken)
.map(Some),
DeviceLoginPollStatus::Expired => Err(BackendAuthClientError::BackendStatus {
status: 410,
body: "device login expired".to_string(),
}),
DeviceLoginPollStatus::Denied => Err(BackendAuthClientError::BackendStatus {
status: 403,
body: "device login was denied".to_string(),
}),
DeviceLoginPollStatus::Consumed => Err(BackendAuthClientError::BackendStatus {
status: 409,
body: "device login was already consumed".to_string(),
}),
DeviceLoginPollStatus::Pending => Ok(None),
}
}
pub async fn wait_for_device_login(
target: &BackendAuthTarget,
device_code: &str,
@@ -99,25 +123,8 @@ pub async fn wait_for_device_login(
let started = std::time::Instant::now();
loop {
let response = poll_device_login(target, device_code).await?;
match response.status {
DeviceLoginPollStatus::Approved => {
return response
.access_token
.ok_or(BackendAuthClientError::MissingAccessToken);
}
DeviceLoginPollStatus::Expired => {
return Err(BackendAuthClientError::BackendStatus {
status: 410,
body: "device login expired".to_string(),
});
}
DeviceLoginPollStatus::Consumed => {
return Err(BackendAuthClientError::BackendStatus {
status: 409,
body: "device login was already consumed".to_string(),
});
}
DeviceLoginPollStatus::Pending => {}
if let Some(access_token) = device_login_poll_result(response)? {
return Ok(access_token);
}
if started.elapsed() >= expires_in {
return Err(BackendAuthClientError::BackendStatus {
@@ -142,3 +149,81 @@ async fn parse_json_response<T: for<'de> Deserialize<'de>>(
}
Ok(response.json::<T>().await?)
}
#[cfg(test)]
mod tests {
use super::*;
use workspace_api::DeviceAccessTokenType;
fn poll_response(status: DeviceLoginPollStatus) -> DeviceLoginPollResponse {
DeviceLoginPollResponse {
status,
access_token: None,
token_type: None,
}
}
#[test]
fn device_login_start_response_enforces_shared_expiry_bounds() {
let valid = serde_json::json!({
"device_code": "device-secret",
"user_code": "ABCD-EFGH",
"verification_uri": "https://yoi.example/login/device",
"verification_uri_complete": "https://yoi.example/login/device?user_code=ABCD-EFGH",
"expires_in": 600,
"interval": 5
});
assert!(serde_json::from_value::<DeviceLoginStartResponse>(valid.clone()).is_ok());
let mut expired = valid;
expired["expires_in"] = serde_json::json!(0);
assert!(serde_json::from_value::<DeviceLoginStartResponse>(expired).is_err());
}
#[test]
fn device_login_poll_response_rejects_unknown_status() {
assert!(
serde_json::from_value::<DeviceLoginPollResponse>(
serde_json::json!({"status": "future_status"}),
)
.is_err()
);
}
#[test]
fn device_login_poll_result_handles_pending_and_terminal_states() {
assert!(
device_login_poll_result(poll_response(DeviceLoginPollStatus::Pending))
.unwrap()
.is_none()
);
let approved = DeviceLoginPollResponse {
status: DeviceLoginPollStatus::Approved,
access_token: Some("access-secret".to_string()),
token_type: Some(DeviceAccessTokenType::Bearer),
};
assert_eq!(
device_login_poll_result(approved).unwrap(),
Some("access-secret".to_string())
);
assert!(matches!(
device_login_poll_result(poll_response(DeviceLoginPollStatus::Approved)),
Err(BackendAuthClientError::MissingAccessToken)
));
for (status, expected_http_status) in [
(DeviceLoginPollStatus::Expired, 410),
(DeviceLoginPollStatus::Denied, 403),
(DeviceLoginPollStatus::Consumed, 409),
] {
assert!(matches!(
device_login_poll_result(poll_response(status)),
Err(BackendAuthClientError::BackendStatus {
status,
..
}) if status == expected_http_status
));
}
}
}
+113
View File
@@ -268,6 +268,7 @@ pub enum DeviceLoginPollStatus {
Pending,
Approved,
Expired,
Denied,
Consumed,
}
@@ -2651,6 +2652,28 @@ mod tests {
.expect("server whoami fixture should match shared DTO");
assert_eq!(serde_json::to_value(decoded).unwrap(), whoami);
let auth_config = serde_json::json!({
"rp_id": "yoi.example",
"origin": "https://yoi.example",
"public_base_url": "https://yoi.example",
"cookie_name": "yoi_workspace_session"
});
let decoded = serde_json::from_value::<AuthPublicConfig>(auth_config.clone())
.expect("server auth-config fixture should match shared DTO");
assert_eq!(serde_json::to_value(decoded).unwrap(), auth_config);
let auth_user = serde_json::json!({
"user": {
"user_id": "user-1",
"account_id": "account-1",
"handle": "hare",
"display_name": "Hare"
}
});
let decoded = serde_json::from_value::<AuthUserResponse>(auth_user.clone())
.expect("server auth-user fixture should match shared DTO");
assert_eq!(serde_json::to_value(decoded).unwrap(), auth_user);
let registration_options = serde_json::json!({
"challenge_id": "challenge-1",
"public_key": {
@@ -2668,6 +2691,65 @@ mod tests {
.expect("server registration options fixture should match shared DTO");
assert_eq!(serde_json::to_value(decoded).unwrap(), registration_options);
let registration_complete = serde_json::json!({
"challenge_id": "challenge-1",
"credential": {
"id": "AQID",
"rawId": "AQID",
"response": {
"attestationObject": "AQID",
"clientDataJSON": "AQID",
"transports": ["internal"]
},
"type": "public-key",
"clientExtensionResults": {},
"authenticatorAttachment": "platform"
}
});
let decoded =
serde_json::from_value::<PasskeyRegistrationCompleteRequest>(registration_complete)
.expect("server registration-complete fixture should match shared DTO");
let encoded = serde_json::to_value(decoded).unwrap();
serde_json::from_value::<PasskeyRegistrationCompleteRequest>(encoded)
.expect("registration-complete DTO should round-trip");
let login_options = serde_json::json!({
"challenge_id": "challenge-2",
"public_key": {
"publicKey": {
"challenge": "AQID",
"rpId": "localhost",
"allowCredentials": [],
"userVerification": "preferred"
}
}
});
let decoded = serde_json::from_value::<PasskeyLoginOptionsResponse>(login_options.clone())
.expect("server login-options fixture should match shared DTO");
assert_eq!(serde_json::to_value(decoded).unwrap(), login_options);
let login_complete = serde_json::json!({
"challenge_id": "challenge-2",
"credential": {
"id": "AQID",
"rawId": "AQID",
"response": {
"authenticatorData": "AQID",
"clientDataJSON": "AQID",
"signature": "AQID",
"userHandle": null
},
"type": "public-key",
"clientExtensionResults": {},
"authenticatorAttachment": "platform"
}
});
let decoded = serde_json::from_value::<PasskeyLoginCompleteRequest>(login_complete)
.expect("server login-complete fixture should match shared DTO");
let encoded = serde_json::to_value(decoded).unwrap();
serde_json::from_value::<PasskeyLoginCompleteRequest>(encoded)
.expect("login-complete DTO should round-trip");
let device_start = serde_json::json!({
"device_code": "device-secret",
"user_code": "ABCD-EFGH",
@@ -2679,6 +2761,37 @@ mod tests {
let decoded = serde_json::from_value::<DeviceLoginStartResponse>(device_start.clone())
.expect("server device-login fixture should match shared DTO");
assert_eq!(serde_json::to_value(decoded).unwrap(), device_start);
let approved_user = AuthenticatedUser {
user_id: "user-1".to_string(),
account_id: "account-1".to_string(),
handle: "hare".to_string(),
display_name: "Hare".to_string(),
};
round_trip(DeviceLoginApproveResponse {
status: DeviceLoginApprovalStatus::Approved,
user: approved_user,
});
for status in [
DeviceLoginPollStatus::Pending,
DeviceLoginPollStatus::Expired,
DeviceLoginPollStatus::Denied,
DeviceLoginPollStatus::Consumed,
] {
round_trip(DeviceLoginPollResponse {
status,
access_token: None,
token_type: None,
});
}
round_trip(DeviceLoginPollResponse {
status: DeviceLoginPollStatus::Approved,
access_token: Some("access-secret".to_string()),
token_type: Some(DeviceAccessTokenType::Bearer),
});
round_trip(LogoutResponse {
status: LogoutStatus::LoggedOut,
});
}
#[test]
+1 -1
View File
@@ -6,7 +6,7 @@
"dev": "deno run -A npm:vite@7.2.7 dev",
"dev:backend": "cd ../.. && cargo run -p yoi-workspace-server --bin yoi-server -- serve --listen 127.0.0.1:8787",
"check": "deno run -A npm:@sveltejs/kit@2.49.4 sync && deno run -A npm:svelte-check@4.3.4 --tsconfig ./tsconfig.json",
"test": "deno test --allow-read=src,test,tests --allow-env=LOG,VSCODE_TEXTMATE_DEBUG,NODE_ENV tests/workspace-model.test.ts tests/workspace-catalog.test.ts tests/profile-api.test.ts src/lib/workspace/auth/model.test.ts src/lib/workspace/api/http.test.ts src/lib/workspace/api/workers.test.ts src/lib/workspace/header/breadcrumb-model.test.ts src/lib/workspace/console/chat-submit.test.ts test/composer-history.test.ts tests/composer-paste.test.ts src/lib/workspace/console/composer-command.test.ts src/lib/workspace/console/composer-draft.test.ts src/lib/workspace/console/composer-completion.test.ts src/lib/workspace/console/markdown.test.ts test/console/ansi.test.ts src/lib/workspace/console/model.test.ts src/lib/workspace/companion/api.test.ts tests/workdir-api.test.ts src/lib/workspace/console/tasks.test.ts test/ticket-detail-route-reuse.test.ts test/repositories/ui.test.ts src/lib/workspace/console/worker-console.ui.test.ts src/lib/workspace/settings/model.test.ts src/lib/workspace/sidebar/override-stack.test.ts src/lib/workspace/sidebar/workers.test.ts src/lib/workspace/sidebar/workspace-switcher.test.ts src/lib/workspace/sidebar/worker-subscription.test.ts src/lib/workspace/sidebar/worker-launch.test.ts test/sidebar/worker-actions.test.ts src/lib/workspace/tickets/merge-request-resources.test.ts src/lib/workspace/tickets/ticket-panel.test.ts test/merge-request-status.test.ts test/config-source/decodal-grammar.test.ts test/config-source/editor-state.test.ts test/config-source/fixed-schema-wrapper.test.ts test/config-source/toolchain.test.ts test/config-source/wasm-parity.test.ts test/repository-access/api.test.ts test/repository-access/loader.test.ts test/repository-access/ui.test.ts",
"test": "deno test --allow-read=src,test,tests --allow-env=LOG,VSCODE_TEXTMATE_DEBUG,NODE_ENV tests/workspace-model.test.ts tests/workspace-catalog.test.ts tests/profile-api.test.ts src/lib/workspace/auth/model.test.ts tests/auth-api.test.ts src/lib/workspace/api/http.test.ts src/lib/workspace/api/workers.test.ts src/lib/workspace/header/breadcrumb-model.test.ts src/lib/workspace/console/chat-submit.test.ts test/composer-history.test.ts tests/composer-paste.test.ts src/lib/workspace/console/composer-command.test.ts src/lib/workspace/console/composer-draft.test.ts src/lib/workspace/console/composer-completion.test.ts src/lib/workspace/console/markdown.test.ts test/console/ansi.test.ts src/lib/workspace/console/model.test.ts src/lib/workspace/companion/api.test.ts tests/workdir-api.test.ts src/lib/workspace/console/tasks.test.ts test/ticket-detail-route-reuse.test.ts test/repositories/ui.test.ts src/lib/workspace/console/worker-console.ui.test.ts src/lib/workspace/settings/model.test.ts src/lib/workspace/sidebar/override-stack.test.ts src/lib/workspace/sidebar/workers.test.ts src/lib/workspace/sidebar/workspace-switcher.test.ts src/lib/workspace/sidebar/worker-subscription.test.ts src/lib/workspace/sidebar/worker-launch.test.ts test/sidebar/worker-actions.test.ts src/lib/workspace/tickets/merge-request-resources.test.ts src/lib/workspace/tickets/ticket-panel.test.ts test/merge-request-status.test.ts test/config-source/decodal-grammar.test.ts test/config-source/editor-state.test.ts test/config-source/fixed-schema-wrapper.test.ts test/config-source/toolchain.test.ts test/config-source/wasm-parity.test.ts test/repository-access/api.test.ts test/repository-access/loader.test.ts test/repository-access/ui.test.ts",
"build": "deno run -A npm:vite@7.2.7 build",
"preview": "deno run -A npm:vite@7.2.7 preview"
},
@@ -93,6 +93,7 @@ export type DeviceLoginPollStatus =
| "pending"
| "approved"
| "expired"
| "denied"
| "consumed";
export type DeviceLoginPollResponse = {
+60 -9
View File
@@ -19,6 +19,63 @@ import {
type WhoamiResponse,
} from "$lib/workspace/auth/model";
const MAX_AUTH_RESPONSE_BYTES = 256 * 1024;
export async function readBoundedAuthResponseJson(
response: Response,
): Promise<unknown> {
const contentLength = response.headers.get("content-length");
if (contentLength !== null) {
const parsed = Number(contentLength);
if (
!Number.isSafeInteger(parsed) || parsed < 0 ||
parsed > MAX_AUTH_RESPONSE_BYTES
) {
await response.body?.cancel();
throw new Error(
"Invalid auth response: response body exceeds the size limit.",
);
}
}
if (response.body === null) {
throw new Error("Invalid auth response: response body is missing.");
}
const reader = response.body.getReader();
const chunks: Uint8Array[] = [];
let totalBytes = 0;
try {
while (true) {
const { done, value } = await reader.read();
if (done) break;
totalBytes += value.byteLength;
if (totalBytes > MAX_AUTH_RESPONSE_BYTES) {
await reader.cancel();
throw new Error(
"Invalid auth response: response body exceeds the size limit.",
);
}
chunks.push(value);
}
} finally {
reader.releaseLock();
}
const bytes = new Uint8Array(totalBytes);
let offset = 0;
for (const chunk of chunks) {
bytes.set(chunk, offset);
offset += chunk.byteLength;
}
try {
return JSON.parse(
new TextDecoder("utf-8", { fatal: true }).decode(bytes),
) as unknown;
} catch {
throw new Error("Invalid auth response: response body is not valid JSON.");
}
}
async function requestJson(path: string, init?: RequestInit): Promise<unknown> {
const response = await fetch(path, {
credentials: "same-origin",
@@ -28,17 +85,11 @@ async function requestJson(path: string, init?: RequestInit): Promise<unknown> {
...(init?.headers ?? {}),
},
});
const body = await response.json() as unknown;
if (!response.ok) {
const errorBody = typeof body === "object" && body !== null
? body as Record<string, unknown>
: null;
const message = typeof errorBody?.message === "string"
? errorBody.message
: `Request failed (${response.status})`;
throw new Error(message);
await response.body?.cancel();
throw new Error(`Auth request failed (${response.status}).`);
}
return body;
return await readBoundedAuthResponseJson(response);
}
export async function loadWhoami(): Promise<WhoamiResponse> {
@@ -147,6 +147,9 @@ Deno.test("device login rejects unsafe expiry and unknown status", () => {
}),
"unsafe expiry",
);
assertEquals(parseDeviceLoginPollResponse({ status: "denied" }), {
status: "denied",
});
assertThrows(
() => parseDeviceLoginPollResponse({ status: "future_status" }),
"unknown device-login status",
@@ -157,6 +160,38 @@ Deno.test("device login rejects unsafe expiry and unknown status", () => {
);
});
Deno.test("auth validation enforces cumulative budgets without echoing unknown keys", () => {
const extensionArrays = Object.fromEntries(
Array.from({ length: 9 }, (_, index) => [
`field-${index}`,
Array.from({ length: 128 }, () => 1),
]),
);
assertThrows(
() =>
prepareLoginOptions({
challenge_id: "challenge-1",
public_key: {
publicKey: {
challenge: "AQID",
extensions: extensionArrays,
},
},
}),
"cumulative value budget",
);
const attackerKey = `secret-${"x".repeat(512)}`;
try {
parseWhoamiResponse({ actor: null, [attackerKey]: true });
throw new Error("expected an error");
} catch (error) {
if (!(error instanceof Error) || error.message.includes(attackerKey)) {
throw new Error("diagnostic included an attacker-controlled key");
}
}
});
Deno.test("passkey credential conversion fails closed on malformed payloads", () => {
const bytes = new Uint8Array([1, 2, 3]).buffer;
const registration = {
+35 -5
View File
@@ -31,6 +31,8 @@ const MAX_AUTH_STRING_LENGTH = 16 * 1024;
const MAX_AUTH_ARRAY_LENGTH = 128;
const MAX_AUTH_OBJECT_KEYS = 128;
const MAX_AUTH_VALUE_DEPTH = 8;
const MAX_AUTH_VALUE_NODES = 1_024;
const MAX_AUTH_VALUE_STRING_UNITS = 64 * 1024;
const MAX_DEVICE_LOGIN_SECONDS = 24 * 60 * 60;
const MAX_WEBAUTHN_TIMEOUT_MS = 10 * 60 * 1000;
const BASE64URL_PATTERN = /^[A-Za-z0-9_-]+={0,2}$/;
@@ -57,7 +59,10 @@ function requireExactKeys(
if (!(key in value)) invalid(`${path}.${key}`, "is required");
}
for (const key of Object.keys(value)) {
if (!allowed.has(key)) invalid(`${path}.${key}`, "is not allowed");
if (key.length === 0 || key.length > 256) {
invalid(path, "has an invalid field name");
}
if (!allowed.has(key)) invalid(path, "contains an unknown field");
}
}
@@ -96,11 +101,30 @@ function positiveSafeInteger(
return value;
}
function boundedJson(value: unknown, path: string, depth = 0): void {
interface ValidationBudget {
remainingNodes: number;
remainingStringUnits: number;
}
function boundedJson(
value: unknown,
path: string,
depth = 0,
budget: ValidationBudget = {
remainingNodes: MAX_AUTH_VALUE_NODES,
remainingStringUnits: MAX_AUTH_VALUE_STRING_UNITS,
},
): void {
budget.remainingNodes -= 1;
if (budget.remainingNodes < 0) invalid(path, "has too many values");
if (depth > MAX_AUTH_VALUE_DEPTH) invalid(path, "is too deeply nested");
if (value === null || typeof value === "boolean") return;
if (typeof value === "string") {
boundedString(value, path, { allowEmpty: true });
budget.remainingStringUnits -= value.length;
if (budget.remainingStringUnits < 0) {
invalid(path, "has too much string data");
}
return;
}
if (typeof value === "number") {
@@ -112,7 +136,7 @@ function boundedJson(value: unknown, path: string, depth = 0): void {
invalid(path, "has too many items");
}
value.forEach((item, index) =>
boundedJson(item, `${path}[${index}]`, depth + 1)
boundedJson(item, `${path}[${index}]`, depth + 1, budget)
);
return;
}
@@ -123,7 +147,11 @@ function boundedJson(value: unknown, path: string, depth = 0): void {
if (key.length === 0 || key.length > 256) {
invalid(path, "has an invalid field name");
}
boundedJson(record[key], `${path}.${key}`, depth + 1);
budget.remainingStringUnits -= key.length;
if (budget.remainingStringUnits < 0) {
invalid(path, "has too much field-name data");
}
boundedJson(record[key], `${path}.field`, depth + 1, budget);
}
}
@@ -683,7 +711,9 @@ export function parseDeviceLoginPollResponse(
]);
const status = response.status;
if (
!["pending", "approved", "expired", "consumed"].includes(String(status))
!["pending", "approved", "expired", "denied", "consumed"].includes(
String(status),
)
) {
invalid("device_login_poll.status", "contains an unknown value");
}
+99
View File
@@ -0,0 +1,99 @@
declare const Deno: {
test(name: string, fn: () => void | Promise<void>): void;
};
import {
loadWhoami,
readBoundedAuthResponseJson,
} from "../src/lib/workspace/auth/api.ts";
function assertEquals(actual: unknown, expected: unknown): void {
if (JSON.stringify(actual) !== JSON.stringify(expected)) {
throw new Error(
`expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`,
);
}
}
async function assertRejects(
promise: Promise<unknown>,
expectedMessage: string,
forbiddenContent?: string,
): Promise<void> {
try {
await promise;
} catch (error) {
if (!(error instanceof Error) || error.message !== expectedMessage) {
throw new Error("unexpected rejection");
}
if (
forbiddenContent !== undefined && error.message.includes(forbiddenContent)
) {
throw new Error("diagnostic leaked response content");
}
return;
}
throw new Error("expected rejection");
}
Deno.test("bounded auth response reader parses a valid JSON object", async () => {
const response = new Response('{"status":"pending"}', {
headers: { "content-type": "application/json" },
});
assertEquals(await readBoundedAuthResponseJson(response), {
status: "pending",
});
});
Deno.test("bounded auth response reader rejects declared and streamed oversize bodies", async () => {
await assertRejects(
readBoundedAuthResponseJson(
new Response("{}", { headers: { "content-length": "262145" } }),
),
"Invalid auth response: response body exceeds the size limit.",
);
const chunk = new Uint8Array(131_073);
const response = new Response(
new ReadableStream<Uint8Array>({
start(controller) {
controller.enqueue(chunk);
controller.enqueue(chunk);
controller.close();
},
}),
);
await assertRejects(
readBoundedAuthResponseJson(response),
"Invalid auth response: response body exceeds the size limit.",
);
});
Deno.test("auth requests do not expose non-success response content", async () => {
const originalFetch = globalThis.fetch;
globalThis.fetch = () =>
Promise.resolve(
new Response('{"message":"access-secret"}', {
status: 401,
headers: { "content-type": "application/json" },
}),
);
try {
await assertRejects(
loadWhoami(),
"Auth request failed (401).",
"access-secret",
);
} finally {
globalThis.fetch = originalFetch;
}
});
Deno.test("bounded auth response reader rejects invalid JSON without echoing content", async () => {
const sensitive = "access-secret";
await assertRejects(
readBoundedAuthResponseJson(new Response(`{${sensitive}`)),
"Invalid auth response: response body is not valid JSON.",
sensitive,
);
});