fix: bound Skill API response handling
This commit is contained in:
@@ -1946,6 +1946,7 @@ pub const SKILL_API_MAX_LABEL_BYTES: usize = 4_096;
|
||||
pub const SKILL_API_MAX_BODY_BYTES: usize = 1_048_576;
|
||||
pub const SKILL_API_MAX_PATH_BYTES: usize = 1_024;
|
||||
pub const SKILL_API_MAX_DIGEST_BYTES: usize = 128;
|
||||
pub const SKILL_API_MAX_RESPONSE_BYTES: usize = 2_097_152;
|
||||
|
||||
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)]
|
||||
#[cfg_attr(feature = "typescript", derive(ts_rs::TS))]
|
||||
@@ -2425,7 +2426,7 @@ pub fn skill_api_typescript() -> String {
|
||||
SkillDetailResponse::decl(&config),
|
||||
];
|
||||
let limits = format!(
|
||||
"export const SKILL_API_AUTHORITY = \"{SKILL_CATALOG_AUTHORITY}\" as const;\n\nexport const SKILL_API_LIMITS = {{\n maxSafeInteger: {SKILL_API_MAX_SAFE_INTEGER},\n maxCatalogEntries: {SKILL_API_MAX_CATALOG_ENTRIES},\n maxOverrides: {SKILL_API_MAX_OVERRIDES},\n maxDiagnostics: {SKILL_API_MAX_DIAGNOSTICS},\n maxResources: {SKILL_API_MAX_RESOURCES},\n maxAllowedTools: {SKILL_API_MAX_ALLOWED_TOOLS},\n maxNameBytes: {SKILL_API_MAX_NAME_BYTES},\n maxLabelBytes: {SKILL_API_MAX_LABEL_BYTES},\n maxBodyBytes: {SKILL_API_MAX_BODY_BYTES},\n maxPathBytes: {SKILL_API_MAX_PATH_BYTES},\n maxDigestBytes: {SKILL_API_MAX_DIGEST_BYTES},\n}} as const;"
|
||||
"export const SKILL_API_AUTHORITY = \"{SKILL_CATALOG_AUTHORITY}\" as const;\n\nexport const SKILL_API_LIMITS = {{\n maxSafeInteger: {SKILL_API_MAX_SAFE_INTEGER},\n maxCatalogEntries: {SKILL_API_MAX_CATALOG_ENTRIES},\n maxOverrides: {SKILL_API_MAX_OVERRIDES},\n maxDiagnostics: {SKILL_API_MAX_DIAGNOSTICS},\n maxResources: {SKILL_API_MAX_RESOURCES},\n maxAllowedTools: {SKILL_API_MAX_ALLOWED_TOOLS},\n maxNameBytes: {SKILL_API_MAX_NAME_BYTES},\n maxLabelBytes: {SKILL_API_MAX_LABEL_BYTES},\n maxBodyBytes: {SKILL_API_MAX_BODY_BYTES},\n maxPathBytes: {SKILL_API_MAX_PATH_BYTES},\n maxDigestBytes: {SKILL_API_MAX_DIGEST_BYTES},\n maxResponseBytes: {SKILL_API_MAX_RESPONSE_BYTES},\n}} as const;"
|
||||
);
|
||||
format!(
|
||||
"// Generated from workspace-api. Do not edit by hand.\n// Regenerate: cargo run -q -p workspace-api --features typescript --example generate_skill_api_types > web/workspace/src/lib/generated/skill-api.ts\n\n{limits}\n\n{}\n",
|
||||
|
||||
@@ -15,6 +15,7 @@ export const SKILL_API_LIMITS = {
|
||||
maxBodyBytes: 1048576,
|
||||
maxPathBytes: 1024,
|
||||
maxDigestBytes: 128,
|
||||
maxResponseBytes: 2097152,
|
||||
} as const;
|
||||
|
||||
export type SkillDiagnosticSeverity = "error" | "warning";
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
import {
|
||||
loadWorkspaceSkillCatalog,
|
||||
loadWorkspaceSkillDetail,
|
||||
workspaceApiPath,
|
||||
workspaceRoute,
|
||||
workspaceSkillActivationPath,
|
||||
workspaceSkillCatalogPath,
|
||||
workspaceSkillDetailPath,
|
||||
} from "./http.ts";
|
||||
import { SKILL_API_LIMITS } from "$lib/generated/skill-api.ts";
|
||||
|
||||
declare const Deno: {
|
||||
test(name: string, fn: () => Promise<void> | void): void;
|
||||
@@ -123,3 +125,44 @@ Deno.test("loadWorkspaceSkillCatalog fetches lightweight catalog", async () => {
|
||||
assertEquals(result.data?.entries[0].name, "triage-errors");
|
||||
assertEquals(JSON.stringify(result.data).includes("SKILL.md body"), false);
|
||||
});
|
||||
|
||||
Deno.test("Skill loaders redact and bound non-success response diagnostics", async () => {
|
||||
const secret = "SENSITIVE-SKILL-BODY-CONTENT".repeat(300);
|
||||
const result = await loadWorkspaceSkillCatalog(
|
||||
(() =>
|
||||
Promise.resolve(new Response(secret, { status: 500 }))) as typeof fetch,
|
||||
"ws-1",
|
||||
);
|
||||
|
||||
assertEquals(result.data, null);
|
||||
assertEquals(result.error, "Skill API request failed with HTTP 500");
|
||||
assert(
|
||||
!result.error?.includes(secret.slice(0, 64)),
|
||||
"Skill API diagnostic must not expose response body content",
|
||||
);
|
||||
assert(
|
||||
(result.error?.length ?? 0) <= 256,
|
||||
"Skill API diagnostic must remain bounded",
|
||||
);
|
||||
});
|
||||
|
||||
Deno.test("Skill loaders stop reading success responses above the wire byte limit", async () => {
|
||||
const oversized = `{"body":"${
|
||||
"x".repeat(SKILL_API_LIMITS.maxResponseBytes + 1)
|
||||
}"}`;
|
||||
const result = await loadWorkspaceSkillDetail(
|
||||
(() =>
|
||||
Promise.resolve(
|
||||
new Response(oversized, { status: 200 }),
|
||||
)) as typeof fetch,
|
||||
"ws-1",
|
||||
"release",
|
||||
);
|
||||
|
||||
assertEquals(result.data, null);
|
||||
assertEquals(result.error, "Skill API response exceeds its byte limit");
|
||||
assert(
|
||||
!result.error?.includes(oversized.slice(0, 64)),
|
||||
"Skill API diagnostic must not expose oversized response content",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { SKILL_API_LIMITS } from "$lib/generated/skill-api.ts";
|
||||
import type {
|
||||
SkillCatalogResponse,
|
||||
SkillDetailResponse,
|
||||
@@ -5,6 +6,7 @@ import type {
|
||||
import {
|
||||
parseSkillCatalogResponse,
|
||||
parseSkillDetailResponse,
|
||||
SkillApiContractError,
|
||||
} from "$lib/workspace/skills/api.ts";
|
||||
|
||||
export type ApiResult<T> = {
|
||||
@@ -14,6 +16,18 @@ export type ApiResult<T> = {
|
||||
|
||||
export type { SkillCatalogResponse, SkillDetailResponse };
|
||||
|
||||
type JsonLoadPolicy = {
|
||||
diagnosticLabel: string;
|
||||
maxResponseBytes: number;
|
||||
};
|
||||
|
||||
const SKILL_API_LOAD_POLICY: JsonLoadPolicy = {
|
||||
diagnosticLabel: "Skill API",
|
||||
maxResponseBytes: SKILL_API_LIMITS.maxResponseBytes,
|
||||
};
|
||||
|
||||
class ResponseByteLimitError extends Error {}
|
||||
|
||||
function normalizePath(path: string): string {
|
||||
if (!path || path === "/") return "";
|
||||
return path.startsWith("/") ? path : `/${path}`;
|
||||
@@ -57,6 +71,7 @@ export async function loadWorkspaceSkillCatalog(
|
||||
workspaceSkillCatalogPath(workspaceId),
|
||||
undefined,
|
||||
parseSkillCatalogResponse,
|
||||
SKILL_API_LOAD_POLICY,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -70,6 +85,7 @@ export async function loadWorkspaceSkillDetail(
|
||||
workspaceSkillDetailPath(workspaceId, name),
|
||||
undefined,
|
||||
parseSkillDetailResponse,
|
||||
SKILL_API_LOAD_POLICY,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -78,19 +94,38 @@ export async function loadJson<T>(
|
||||
path: string,
|
||||
init?: RequestInit,
|
||||
parse: (value: unknown) => T = (value) => value as T,
|
||||
policy?: JsonLoadPolicy,
|
||||
): Promise<ApiResult<T>> {
|
||||
try {
|
||||
const response = await fetchFn(path, init);
|
||||
if (!response.ok) {
|
||||
if (policy) {
|
||||
await response.body?.cancel();
|
||||
return {
|
||||
data: null,
|
||||
error:
|
||||
`${policy.diagnosticLabel} request failed with HTTP ${response.status}`,
|
||||
};
|
||||
}
|
||||
const text = await response.text();
|
||||
return {
|
||||
data: null,
|
||||
error: text || `${path} request failed (${response.status})`,
|
||||
};
|
||||
}
|
||||
const payload: unknown = await response.json();
|
||||
const payload: unknown = policy
|
||||
? await readBoundedJson(response, policy.maxResponseBytes)
|
||||
: await response.json();
|
||||
return { data: parse(payload), error: null };
|
||||
} catch (error) {
|
||||
if (policy) {
|
||||
const diagnostic = error instanceof SkillApiContractError
|
||||
? error.message
|
||||
: error instanceof ResponseByteLimitError
|
||||
? `${policy.diagnosticLabel} response exceeds its byte limit`
|
||||
: `${policy.diagnosticLabel} response is invalid`;
|
||||
return { data: null, error: diagnostic.slice(0, 256) };
|
||||
}
|
||||
return {
|
||||
data: null,
|
||||
error: error instanceof Error ? error.message : `${path} request failed`,
|
||||
@@ -98,6 +133,50 @@ export async function loadJson<T>(
|
||||
}
|
||||
}
|
||||
|
||||
async function readBoundedJson(
|
||||
response: Response,
|
||||
maxBytes: number,
|
||||
): Promise<unknown> {
|
||||
const contentLength = response.headers.get("content-length");
|
||||
if (contentLength !== null) {
|
||||
const parsedLength = Number(contentLength);
|
||||
if (Number.isFinite(parsedLength) && parsedLength > maxBytes) {
|
||||
await response.body?.cancel();
|
||||
throw new ResponseByteLimitError();
|
||||
}
|
||||
}
|
||||
|
||||
const reader = response.body?.getReader();
|
||||
if (!reader) {
|
||||
throw new Error("response body is unavailable");
|
||||
}
|
||||
const chunks: Uint8Array[] = [];
|
||||
let totalBytes = 0;
|
||||
try {
|
||||
while (true) {
|
||||
const { done, value } = await reader.read();
|
||||
if (done) break;
|
||||
totalBytes += value.byteLength;
|
||||
if (totalBytes > maxBytes) {
|
||||
await reader.cancel();
|
||||
throw new ResponseByteLimitError();
|
||||
}
|
||||
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;
|
||||
}
|
||||
const text = new TextDecoder("utf-8", { fatal: true }).decode(bytes);
|
||||
return JSON.parse(text) as unknown;
|
||||
}
|
||||
|
||||
async function requireJson<T>(response: Response, path: string): Promise<T> {
|
||||
if (!response.ok) {
|
||||
const text = await response.text();
|
||||
|
||||
Reference in New Issue
Block a user