diff --git a/apps/web/app/lib/personal-kits/kit-http.ts b/apps/web/app/lib/personal-kits/kit-http.ts index 8c484ee..e8afa3e 100644 --- a/apps/web/app/lib/personal-kits/kit-http.ts +++ b/apps/web/app/lib/personal-kits/kit-http.ts @@ -30,11 +30,12 @@ export const kitNotFound = () => releaseError(404, "not_found", "Not found"); export const badRequest = (message: string) => releaseError(400, "invalid_request", message); export const conflict = (code: string, message: string) => releaseError(409, code, message); -/** Review and activation of a session-derived revision: a human in a web session, never an API key. */ -export function requireInteractive(caller: { interactive: boolean }): void { - if (!caller.interactive) { - throw releaseError(403, "kit.interactive_approval_required", "approve session-derived revisions signed in on the web, not with an API key"); - } +/** + * Review and activation of a session-derived revision, and any activation + * that makes a kit public: a human in a web session, never an API key. + */ +export function requireInteractive(caller: { interactive: boolean }, message = "approve session-derived revisions signed in on the web, not with an API key"): void { + if (!caller.interactive) throw releaseError(403, "kit.interactive_approval_required", message); } /** `@handle/name` of the route; malformed is 400. */ diff --git a/apps/web/app/lib/personal-kits/kit-lifecycle.ts b/apps/web/app/lib/personal-kits/kit-lifecycle.ts index c903d23..6a9cc15 100644 --- a/apps/web/app/lib/personal-kits/kit-lifecycle.ts +++ b/apps/web/app/lib/personal-kits/kit-lifecycle.ts @@ -4,8 +4,11 @@ * exact `revisionDigest`, re-checks every base of the effective lineage and * moves `effective_revision_id` under the generation guard; nothing becomes * effective implicitly. A proposal (`proposed`) must be reviewed first, and - * a session-derived revision activates only on a private kit and only from - * an interactive web session. A quarantined revision anywhere in the chain + * a revision with a session-derived revision anywhere in its history (its + * revision chain, any origin, and the revision chain of every parent revision + * its document lineage names; an unreadable parent counts) activates only on a private kit, + * as a private document, and only from an interactive web session. Any + * activation that makes a kit public needs a web session too. A quarantined revision anywhere in the chain * fails the lock with `kit.locked_incompatible`. * Rejection is final for that revision; a proposal is rejected only through * its web-session review, never through the generic reject. The lock is derived on read from the @@ -20,6 +23,7 @@ import type { KitCaller } from "./kit-http"; import { commitKitMutation, documentOf, getRevision, ownedKit, writableKit } from "./kit-store"; import type { KitRow, RevisionRow } from "./kit-store"; import { chainView, loadChain } from "./kit-verify"; +import type { LoadedChain } from "./kit-verify"; import { generationConflict } from "./kit-revisions"; const ACTIVATABLE = new Set(["draft", "reviewed"]); @@ -27,6 +31,8 @@ const ACTIVATABLE = new Set(["draft", "reviewed"]); interface LockCheck { /** Null when the lineage is incomplete; `reasons` then says why. */ lock: Awaited> | null; + /** The parent revisions the document lineage loaded. */ + parents: LoadedChain["parents"]; documents: unknown[]; reasons: string[]; warnings: string[]; @@ -36,7 +42,7 @@ interface LockCheck { export async function checkLock(caller: KitCaller, rev: RevisionRow): Promise { const reasons: string[] = rev.state === "quarantined" ? ["the revision is quarantined"] : []; const loaded = await loadChain(caller, documentOf(rev), reasons); - if (loaded.incomplete) return { lock: null, documents: [], reasons, warnings: [] }; + if (loaded.incomplete) return { lock: null, parents: loaded.parents, documents: [], reasons, warnings: [] }; const { chain } = loaded; const warnings: string[] = []; for (const { entry } of resolveLineage(chain)) { @@ -50,7 +56,7 @@ export async function checkLock(caller: KitCaller, rev: RevisionRow): Promise { + if (rev.origin === "session-derived") return true; + const row = await db + .prepare( + `WITH RECURSIVE chain(id, origin, parent) AS ( + SELECT id, origin, parent_revision_id FROM personal_kit_revisions WHERE id = ? AND kit_id = ? + UNION + SELECT r.id, r.origin, r.parent_revision_id FROM personal_kit_revisions r JOIN chain c ON r.id = c.parent WHERE r.kit_id = ?) + SELECT COUNT(*) AS n, + COALESCE(SUM(origin = 'session-derived'), 0) AS tainted, + COALESCE(SUM(parent IS NOT NULL AND parent NOT IN (SELECT id FROM chain)), 0) AS dangling + FROM chain`, + ) + .bind(rev.id, kitId, kitId) + .first<{ n: number; tainted: number; dangling: number }>(); + return row === null || Number(row.n) === 0 || Number(row.tainted) > 0 || Number(row.dangling) > 0; +} + /** `POST .../activate` and `POST .../rollback` (rollback: a revision that was effective before). */ export async function activateRevision(caller: KitCaller, kitName: string, body: Record, generation: number, rollback: boolean): Promise { const kit = await writableKit(caller.env.DB, kitName, caller.userId); @@ -71,17 +101,26 @@ export async function activateRevision(caller: KitCaller, kitName: string, body: if (rev.state === "proposed") throw conflict("kit.review_required", "a proposed revision must be reviewed before activation"); if (!ACTIVATABLE.has(rev.state)) throw conflict("kit.revision_unusable", `the revision is ${rev.state}`); const sessionDerived = rev.origin === "session-derived"; - if (sessionDerived) { - if (rev.state !== "reviewed") throw conflict("kit.review_required", "a session-derived revision must be reviewed"); + if (sessionDerived && rev.state !== "reviewed") throw conflict("kit.review_required", "a session-derived revision must be reviewed"); + const doc = documentOf(rev); + // The lock loads the document lineage: a parent revision in another kit carries its own history. + const check = await checkLock(caller, rev); + const histories = [{ kitId: kit.id, row: rev }, ...check.parents]; + let gated = false; + for (const { kitId, row } of histories) if (!gated) gated = await derivesFromSession(caller.env.DB, kitId, row); + if (gated) { requireInteractive(caller); - // Activation would make the kit private; a public kit is forked to a private one instead. - if (kit.visibility !== "private") throw conflict("proposal.private_kit_required", "a session-derived revision activates on a private kit only; fork this kit to a private kit"); + // Both the kit and the visibility the activation writes stay private; a public kit is forked to a private one instead. + if (kit.visibility !== "private" || doc.visibility !== "private") { + throw conflict("proposal.private_kit_required", "a revision derived from a session-derived revision activates on a private kit, as a private document, only"); + } } + // Publishing is a human decision whatever the origin: an API key never makes a kit public. + if (doc.visibility !== "private") requireInteractive(caller, "activate a public revision signed in on the web, not with an API key"); if (rollback && (rev.activated_at === null || rev.id === kit.effective_revision_id)) throw badRequest("rollback targets an earlier effective revision"); if (!rollback && rev.id === kit.effective_revision_id) throw badRequest("the revision is already effective"); - const check = await checkLock(caller, rev); + // A parent that could not be loaded fails here, so the gate never trusts a lineage it did not read. if (check.reasons.length > 0) return incompatible(check.reasons); - const doc = documentOf(rev); const at = caller.now.getTime(); // A reviewed proposal keeps its review proof next to the activation approval. const review = sessionDerived && rev.approval_json !== null ? (JSON.parse(rev.approval_json) as { review?: unknown }) : null; diff --git a/apps/web/app/lib/personal-kits/kit-revisions.ts b/apps/web/app/lib/personal-kits/kit-revisions.ts index 2483840..7be2d22 100644 --- a/apps/web/app/lib/personal-kits/kit-revisions.ts +++ b/apps/web/app/lib/personal-kits/kit-revisions.ts @@ -77,7 +77,11 @@ export async function readRevision(caller: KitCaller, kitName: string, id: strin return json(revisionView(await getRevision(caller.env.DB, kit.id, id), true)); } -/** Validates, verifies and stores one revision under the generation guard. */ +/** + * Validates, verifies and stores one revision under the generation guard. A + * new kit always starts private: only an activation sets its visibility, so + * an API key never creates a public kit. + */ export async function storeRevision(caller: KitCaller, req: StoreRequest): Promise<{ revision: RevisionRow; generation: number; warnings: string[]; replayed: boolean }> { const db = caller.env.DB; const existing = await findKit(db, req.kitName); @@ -107,7 +111,7 @@ export async function storeRevision(caller: KitCaller, req: StoreRequest): Promi const ok = await commitKitMutation(db, { kitId, generation: req.expectedGeneration, - create: existing === null ? { name: req.kitName, publisherId, ownerUserId: caller.userId, localId: doc.localId ?? null, visibility: doc.visibility } : undefined, + create: existing === null ? { name: req.kitName, publisherId, ownerUserId: caller.userId, localId: doc.localId ?? null, visibility: "private" } : undefined, statements: [ { sql: `INSERT INTO personal_kit_revisions (id, kit_id, revision_number, parent_revision_id, revision_digest, document_json, state, origin, diff --git a/apps/web/app/lib/personal-kits/kit-verify.ts b/apps/web/app/lib/personal-kits/kit-verify.ts index e993263..5171a54 100644 --- a/apps/web/app/lib/personal-kits/kit-verify.ts +++ b/apps/web/app/lib/personal-kits/kit-verify.ts @@ -62,6 +62,8 @@ export interface LoadedChain { foreign: Set; /** A parent was unavailable (reported in `reasons`), so `chain` stops before it. */ incomplete: boolean; + /** The parent revision rows behind `chain[1..]`, with the id of their kit. */ + parents: Array<{ kitId: string; row: RevisionRow }>; } /** @@ -76,6 +78,7 @@ export async function loadChain(caller: KitCaller, doc: PersonalKitDocument, rea const { env, userId } = caller; const chain = [doc]; const foreign = new Set(); + const parents: LoadedChain["parents"] = []; let parentPublic = true; for (let cur = doc; cur.parent !== undefined; cur = chain[chain.length - 1]!) { if (chain.length >= MAX_LINEAGE_DEPTH || chain.some((d, i) => i > 0 && d.kit === cur.parent!.kit) || cur.parent.kit === doc.kit) { @@ -92,14 +95,15 @@ export async function loadChain(caller: KitCaller, doc: PersonalKitDocument, rea const detail = `parent revision ${cur.parent.revisionDigest} of ${cur.parent.kit} is not available`; if (reasons === undefined) throw kitError(KIT_CODES.baseUnavailable, "parent", detail); reasons.push(detail); - return { chain, parentPublic: false, foreign, incomplete: true }; + return { chain, parentPublic: false, foreign, incomplete: true, parents }; } if (kit!.visibility !== "public") parentPublic = false; if (!owned) foreign.add(chain.length); + parents.push({ kitId: kit!.id, row: row! }); chain.push(documentOf(row!)); } resolveLineage(chain); - return { chain, parentPublic, foreign, incomplete: false }; + return { chain, parentPublic, foreign, incomplete: false, parents }; } /** The chain as returned to the caller: own documents in full, foreign ancestors as digest references only. */ diff --git a/apps/web/app/lib/personal-kits/proposal-detectors.ts b/apps/web/app/lib/personal-kits/proposal-detectors.ts index f5153a3..40d4352 100644 --- a/apps/web/app/lib/personal-kits/proposal-detectors.ts +++ b/apps/web/app/lib/personal-kits/proposal-detectors.ts @@ -1,118 +1,132 @@ /** - * Text detectors for session-derived content. Every absolute local path is - * refused, in plain, JSON-escaped (`\/`, `\\`) and percent-encoded (`%2F`, - * `%5C`, `%3A`) form, case-insensitively: a drive-letter path, a UNC share - * (`\\host\share` or `//host/share`), a `file:` or `smb:` URL, a bare user, - * temp, mount or volume root (`/home`, `/Users`, `/tmp`, ...) and any POSIX - * path of two or more segments. Only well-known system paths stay allowed: - * `/usr/`, `/bin/`, `/sbin/`, `/etc/`, `/proc/` and `/dev/null`, `/dev/stdin`, - * `/dev/stdout`, `/dev/stderr`. Web URLs (`https://host/path`) are not paths. - * Generic credentials (password assignments, also a YAML `password:` key - * whose value sits on the next lines, Bearer tokens, URLs with `user:pass@`, - * JWTs, private key blocks and dotenv-style secret keys) complement the - * vendor token rules of the release scanner. Detectors return rule names - * only, never the matched text. + * Text detectors for session-derived content. Absolute local paths are + * refused by `proposal-path-detector`. Generic credentials complement the + * vendor token rules of the release scanner: + * - a `key: value`, `key = value` or `"key": "value"` assignment (YAML, JSON, + * code, `.env`) whose key names a password or a secret (`dbPassword`, + * `client_secret`, `apiKey`, `GITHUB_TOKEN`), compared through the shared + * key normalization of `@skillx/contracts`, so content rules and envelope + * keys agree; a YAML password key whose value sits on the following lines; + * - Bearer tokens, URLs with `user:pass@`, JWTs and private key blocks. + * Detectors return rule names only, never the matched text. Every pattern + * runs in time linear in the text (bounded keys and scheme names, one start + * per token or line), so hostile input cannot exhaust the Worker CPU. The + * shared vectors are in `@skillx/contracts` (`kit-proposal-policy-vectors-v1.json`). */ +import { secretKeyKind } from "@skillx/contracts"; -/** Not part of a path or word: what may precede an absolute path. */ -const START = String.raw`(? = [ - // Drive letter: C:\, C:/, C:\\ (JSON-escaped), with or without a following segment. - new RegExp(String.raw`${START}[A-Za-z]:${SEP}`), - // UNC share (\\host\share), also escaped (\\\\host\\share). - new RegExp(String.raw`(?`, `%VAR%`, `{...}`, `***`, `${...}` or + * a whole `$VAR`. Any other value, also one that starts with `$`, is real. + */ +const PLACEHOLDER = String.raw`(?:[<%{*]|\$\{|\$[A-Z_][A-Z0-9_]*(?![^\s"'<>]))`; +/** Up to two YAML anchors or tags before the value (`&a`, `!secret`, `&a !s`). */ +const ANCHOR = String.raw`(?:[&!][^\s"'<>]{0,64}[ \t]+){0,2}`; +/** The same before a block value, where an anchor or tag may end the key line. */ +const BLOCK_ANCHOR = String.raw`(?:[&!][^\s"'<>]{0,64}[ \t]*){0,2}`; +/** + * `key value` on one line (`:`, `=`, `:=`, `=>`): a key of at most 128 + * characters, optionally quoted or in brackets (`cfg["password"]`), at a key + * boundary (also after `--` of a flag); group 1 is the key, 2 the separator, + * 3 the value. The value (at most 256 characters) is read in a lookahead, so + * the scan resumes right after the separator and finds a key inside a value + * (`Server=db;Password=...`, `?user=a&password=...`), and each start reads a + * bounded window. A key past the 64-character policy limit counts as a secret key. + */ +const ASSIGNMENT = new RegExp( + String.raw`(?|[:=])[ \t]*${ANCHOR}["']?(?=(?!${PLACEHOLDER})([^\s"'<>]{1,256}))`, + "g", +); +/** A `KEY=value` (or `export KEY=value`) line start before the key at the end of this window. */ +const DOTENV_START = /(?:^|\n)[ \t]*(?:export[ \t]+)?["']?$/; +const MIN_PASSWORD = 4; +const MIN_SECRET = 8; -/** A POSIX path of two or more segments (also `//host/share`); groups: first segment, second segment. */ -const POSIX_PATH = new RegExp(String.raw`${START}${SLASHES}(${SEGMENT})${SLASHES}(${SEGMENT})?`, "gi"); -const SYSTEM_ROOTS = new Set(["usr", "bin", "sbin", "etc", "proc"]); -const SYSTEM_DEVICES = new Set(["null", "stdin", "stdout", "stderr"]); -/** A web URL (`scheme://...`); its path is not a local path. `file:` and `smb:` are refused before this runs. */ -const WEB_URL = /\b[a-z][a-z0-9+.-]*:(?:\\*\/){2,}[^\s"'<>`]*/gi; +const LINE_RULES: ReadonlyArray<[rule: string, re: RegExp]> = [ + ["secret-bearer", /\bbearer\s+[A-Za-z0-9._~+/-]{16,}=*/i], + ["secret-url-credentials", /\b[a-z][a-z0-9+.-]{0,31}:\/\/[^\s/?#:@]+:[^\s/?#@]+@/i], + ["secret-private-key", /-----BEGIN (?:[A-Z0-9]+ )*PRIVATE KEY(?: BLOCK)?-----/], +]; -/** Decodes percent-encoded separators and colons (also double-encoded, `%252F`). */ -function decodeSeparators(text: string): string { - let out = text; - for (let i = 0; i < 3; i++) { - const next = out.replace(/%25(2f|5c|3a)/gi, "%$1").replace(/%2f/gi, "/").replace(/%5c/gi, "\\").replace(/%3a/gi, ":"); - if (next === out) break; - out = next; +/** + * A JWT: `eyJ` at a word boundary, at least four base64url characters, a + * dot, `eyJ` and four more, and another dot. Checked per token and part so + * a long run of `eyJ-eyJ-...` is read once. + */ +function hasJwt(text: string): boolean { + for (const [token] of text.matchAll(/[A-Za-z0-9_.-]+/g)) { + if (!token.includes("eyJ")) continue; + const parts = token.split("."); + for (let i = 0; i + 2 < parts.length; i++) { + const start = /(?:^|-)eyJ/.exec(parts[i]!); + const head = start === null ? -1 : start.index + start[0].length; + if (head >= 0 && parts[i]!.length - head >= 4 && parts[i + 1]!.startsWith("eyJ") && parts[i + 1]!.length >= 7) return true; + } } - return out; -} - -function allowedSystemPath(path: string, first: string, second: string | undefined, after: string): boolean { - const root = first.toLowerCase(); - // `/usr/bin/../../home/...` climbs out of the system root. - const whole = path + /^[A-Za-z0-9._~+@$%\\/-]*/.exec(after)![0]; - if (/(?:^|[\\/])\.\.(?:[\\/]|$)/.test(whole)) return false; - if (SYSTEM_ROOTS.has(root)) return true; - // `/dev/null` and the standard streams, but not a path below them. - return root === "dev" && second !== undefined && SYSTEM_DEVICES.has(second.toLowerCase()) && !/^\\*\//.test(after); + return false; } -/** True when `text` names an absolute local path. */ -export function hasLocalPath(text: string): boolean { - const decoded = decodeSeparators(text); - if (LOCAL_PATHS.some((re) => re.test(decoded))) return true; - const withoutUrls = decoded.replace(WEB_URL, " "); - for (const m of withoutUrls.matchAll(POSIX_PATH)) { - if (!allowedSystemPath(m[0], m[1]!, m[2], withoutUrls.slice(m.index! + m[0].length))) return true; +/** + * Rules of the assignments in `text`: `secret-password` for a password key + * (value of 4 or more characters); for other keys, values of 8 or more + * characters, `secret-dotenv` on a `KEY=value` line whose key holds a + * password or secret stem anywhere, `secret-assignment` for any other + * assignment to a secret key. + */ +function assignmentRules(text: string, rules: Set): void { + for (const m of text.matchAll(ASSIGNMENT)) { + const [, key, sep, value] = m as unknown as [string, string, string, string]; + if (/^[0-9.]+$/.test(value)) continue; + const kind = secretKeyKind(key); + if (kind === "password") { + if (value.length >= MIN_PASSWORD) rules.add("secret-password"); + continue; + } + if (value.length < MIN_SECRET) continue; + const dotenv = sep === "=" && DOTENV_START.test(text.slice(Math.max(0, m.index! - 80), m.index!)); + if (dotenv && secretKeyKind(key, true) !== null) rules.add("secret-dotenv"); + else if (kind === "secret") rules.add("secret-assignment"); } - return false; } -const SECRET_KEY = "[A-Za-z0-9_]*(?:SECRET|TOKEN|PASSWORD|PASSWD|PWD|API_?KEY|ACCESS_?KEY|PRIVATE_?KEY|CLIENT_?SECRET|CREDENTIALS?)[A-Za-z0-9_]*"; -/** A value that is not an obvious placeholder (`<...>`, `${...}`, `$VAR`, `%VAR%`, `***`). */ -const VALUE = String.raw`["']?(?![<$%{*])[^\s"'<>]`; - -const GENERIC_SECRETS: ReadonlyArray<[rule: string, re: RegExp]> = [ - ["secret-password", new RegExp(String.raw`(? re.test(text)).map(([rule]) => rule).sort(); + const rules = new Set(LINE_RULES.filter(([, re]) => re.test(text)).map(([rule]) => rule)); + if (hasJwt(text)) rules.add("secret-jwt"); + assignmentRules(text, rules); + return [...rules].sort(); } /** - * A YAML password key whose value sits on the following, more indented lines - * (a plain indented block or a `|` / `>` block scalar). Group 2 is the block. + * A YAML password key (also a list item `- password:` or a quoted key + * `"password":`) whose value sits on the following, more indented lines (a + * plain indented block or a `|` / `>` block scalar). Group 2 is the block. */ const YAML_PASSWORD_BLOCK = new RegExp( - String.raw`(?:^|\n)([ \t]*)[A-Za-z0-9_-]*(?:password|passwd|passphrase|pwd)["']?[ \t]*:[ \t]*(?:[|>][-+0-9]*[ \t]*)?(?:#[^\n]*)?\n((?:[ \t]*\n)*\1[ \t]+\S[^\n]*(?:\n\1[ \t]+[^\n]*|\n[ \t]*(?=\n))*)`, + String.raw`(?:^|\n)([ \t]*)(?:-[ \t]+)*["']?[A-Za-z0-9_-]*(?:password|passwd|passphrase|pwd)["']?[ \t]*:[ \t]*${BLOCK_ANCHOR}(?:[|>][-+0-9]*[ \t]*)?(?:#[^\n]*)?\n((?:[ \t]*\n)*\1[ \t]+\S[^\n]*(?:\n\1[ \t]+[^\n]*|\n[ \t]*(?=\n))*)`, "gi", ); -/** An obvious placeholder value (`<...>`, `${...}`, `$VAR`, `%VAR%`, `***`), optionally quoted. */ -const PLACEHOLDER = /^["']?[<$%{*]/; +const BLOCK_PLACEHOLDER = new RegExp(String.raw`^["']?${PLACEHOLDER}`); + +/** The password blocks of `text` that hold a real value. */ +function passwordBlocks(text: string): string[] { + const blocks: string[] = []; + for (const m of text.matchAll(YAML_PASSWORD_BLOCK)) { + const value = m[2]!.trim(); + if (value.length >= MIN_PASSWORD && !BLOCK_PLACEHOLDER.test(value)) blocks.push(m[0].replace(/^\n/, "")); + } + return blocks; +} /** * True when `after` holds a YAML password block that `before` does not: the * line-based diff alone would miss a new value under a key the base already has. */ export function hasMultilinePassword(before: string | null, after: string): boolean { - for (const m of after.matchAll(YAML_PASSWORD_BLOCK)) { - const value = m[2]!.trim(); - if (value.length < 4 || PLACEHOLDER.test(value)) continue; - if (before === null || !before.includes(m[0].replace(/^\n/, ""))) return true; - } - return false; + const kept = new Set(before === null ? [] : passwordBlocks(before)); + return passwordBlocks(after).some((block) => !kept.has(block)); } /** True for bytes that open with a UTF-8, UTF-16 or UTF-32 byte order mark. */ diff --git a/apps/web/app/lib/personal-kits/proposal-envelope.ts b/apps/web/app/lib/personal-kits/proposal-envelope.ts index 168083c..488d032 100644 --- a/apps/web/app/lib/personal-kits/proposal-envelope.ts +++ b/apps/web/app/lib/personal-kits/proposal-envelope.ts @@ -7,10 +7,14 @@ * * Raw session data is never accepted: a key that names a transcript, * prompt, message list, environment, secret, working directory or history - * anywhere in the proposal is refused with `proposal.forbidden_field` - * naming the key path, never its value. Unknown fields are refused too. + * anywhere in the proposal (in any case, separator or encoding, also inside + * a string that holds JSON) is refused with `proposal.forbidden_field` + * naming the key path, never its value; so is a key that cannot be checked + * (non-ASCII or overlong). The rules are the shared, versioned + * `skillx.kit-proposal-policy/v1` that AgentKit applies before upload. + * Unknown fields are refused too. */ -import { validateOverlay } from "@skillx/contracts"; +import { findForbiddenProposalKey, validateOverlay } from "@skillx/contracts"; import type { KitSkillEntry, SkillOverlay } from "@skillx/contracts"; import { releaseError } from "~/lib/releases/release-http"; import { MAX_JSON_DEPTH, requireId } from "./kit-http"; @@ -25,29 +29,20 @@ export interface KitProposal { evidence: { summary: string; signals: Record } | null; } -const FORBIDDEN_KEYS = new Set([ - "transcript", "transcripts", "session", "sessions", "sessionid", "sessionlog", "prompt", "prompts", "systemprompt", - "messages", "conversation", "chat", "env", "environment", "envvars", "secret", "secrets", "token", "tokens", "apikey", - "credentials", "password", "cwd", "workdir", "homedir", "history", "toolcalls", "tooloutput", "stdout", "stderr", "logs", "raw", -]); - const isRecord = (v: unknown): v is Record => typeof v === "object" && v !== null && !Array.isArray(v); export const proposalError = (code: string, message: string) => releaseError(422, code, message); const invalid = (message: string) => proposalError("proposal.invalid", message); -/** Refuses raw session data by key name at any depth; reports the key path only. */ -export function assertNoForbiddenKeys(value: unknown, at = "proposal", depth = 0): void { - if (typeof value === "object" && value !== null && depth >= MAX_JSON_DEPTH) throw invalid(`${at} nests deeper than ${MAX_JSON_DEPTH} levels`); - if (Array.isArray(value)) return value.forEach((v, i) => assertNoForbiddenKeys(v, `${at}[${i}]`, depth + 1)); - if (!isRecord(value)) return; - for (const [key, child] of Object.entries(value)) { - const path = `${at}.${key}`; - // A number (an evidence signal such as `sessions: 3`) cannot carry session content. - if (typeof child !== "number" && FORBIDDEN_KEYS.has(key.toLowerCase().replace(/[-_]/g, ""))) { - throw proposalError("proposal.forbidden_field", `${path} is not accepted: proposals carry reviewed overlays only, never raw session data`); - } - assertNoForbiddenKeys(child, path, depth + 1); - } +/** + * Refuses raw session data by key name at any depth, with the shared + * `skillx.kit-proposal-policy/v1` rules (see `@skillx/contracts`); reports + * the key path only. + */ +export function assertNoForbiddenKeys(value: unknown): void { + const found = findForbiddenProposalKey(value, MAX_JSON_DEPTH); + if (found === null) return; + if (found.reason === "too_deep") throw invalid(`${found.path} nests deeper than ${MAX_JSON_DEPTH} levels`); + throw proposalError("proposal.forbidden_field", `${found.path} is not accepted: proposals carry reviewed overlays only, never raw session data`); } function only(raw: Record, at: string, fields: string[]): void { diff --git a/apps/web/app/lib/personal-kits/proposal-path-detector.ts b/apps/web/app/lib/personal-kits/proposal-path-detector.ts new file mode 100644 index 0000000..af50770 --- /dev/null +++ b/apps/web/app/lib/personal-kits/proposal-path-detector.ts @@ -0,0 +1,122 @@ +/** + * Absolute local path detector for session-derived content. Every absolute + * local path is refused, in plain, JSON-escaped (`\/`, `\\`) and + * percent-encoded (`%2F`, `%5C`, `%3A`, `%2E`, up to three rounds; an + * encoded separator or dot left after them refuses the text) form, + * case-insensitively, after NFC normalization, with Unicode slash forms + * (`／`, `∕`, `⁄`, `＼`) read as separators and invisible format characters + * (zero-width spaces and joiners) removed: a drive-letter path, a UNC share (`\\host\share` + * or `//host/share`), a `file:` or `smb:` URL, an editor link to a file + * (`vscode://file/...`), a user home in an `ssh`/`sftp` URL, a bare user, + * temp, mount or volume root (`/home`, `/Users`, `/tmp`, ...), a path glued to + * a compiler flag (`-I/opt/x`) and any POSIX path of two or more segments, in + * any script (`/Usuários/...`). Only the shared system allowlist stays + * allowed: whole subtrees of binary and library directories (`/usr/bin/...`, + * `/usr/local/lib/...`) and exact well-known files and directories + * (`/etc/hosts`, `/proc/cpuinfo`, `/dev/null`, ...). URLs of network schemes + * (`https://host/path`) are not paths. + * + * Every pattern runs in time linear in the text: separator runs are + * collapsed first, scheme names are bounded and each path run is read once, + * so a long run of `/`, `\` or `a-a-a-...` cannot exhaust the Worker CPU. + */ +import { KIT_PROPOSAL_POLICY } from "@skillx/contracts"; + +/** Not part of a path or word: what may precede an absolute path. */ +const START = String.raw`(? = [ + // Drive letter: C:\, C:/, C:\\ (JSON-escaped), with or without a following segment. + new RegExp(String.raw`${START}[A-Za-z]:${SEP}`, "u"), + // UNC share (\\host\share), also escaped (\\\\host\\share). + new RegExp(String.raw`(?`]*(?:\\*\/)+(?:home|Users|root)(?![A-Za-z0-9_-])/i, + // A bare user, temp, mount or volume root, after any non-path character (also `\/home`). + new RegExp(String.raw`${START}${SEP}(?:home|Users|root|mnt|media|Volumes|var${SEP}folders|tmp|private|run${SEP}user)(?![\p{L}\p{N}_-])`, "iu"), +]; + +/** A POSIX path of two or more segments (also `//host/share`). */ +const POSIX_PATH = new RegExp(String.raw`${START}${SLASHES}${SEGMENT}${SLASHES}(?:${SEGMENT})?`, "giu"); +/** The path run a match starts. */ +const PATH_RUN = new RegExp(String.raw`[${SEG_CHARS}\\/]*`, "uy"); +const DOT_DOT = /(?:^|\/)\.\.(?=\/|$)/; + +/** The shared system allowlist of `skillx.kit-proposal-policy/v1`. */ +const ALLOW = KIT_PROPOSAL_POLICY.content.localPathSystemAllowlist; +const EXACT = new Set(ALLOW.exact); +const SUBTREES = ALLOW.subtrees; + +/** A URL of a network scheme; its path is not a local path. Other schemes (`vscode:`, `chrome-extension:`) keep theirs. */ +const WEB_URL = + /(?`,;|&{}()[\]]*/gi; +/** A short flag glued to a path (`-I/opt/x`, `-isystem/opt/x`), after a word boundary of a command line. */ +const GLUED_FLAG = /(? text.replace(/[\uff0f\u2215\u2044\u29f8]/g, "/").replace(/[\uff3c\u2216\u29f9]/g, "\\"); + +/** Shortens every run of more than 8 separators to its first and last 4, which every pattern reads the same way. */ +const collapseSeparators = (text: string) => text.replace(/[\\/]{9,}/g, (run) => run.slice(0, 4) + run.slice(-4)); + +/** + * True for a path run on the allowlist: compared in lower case with every + * separator run as one `/` and trailing separators and dots removed, it + * equals an exact entry or lies inside a subtree; a `..` segment refuses it. + */ +function allowedSystemPath(run: string): boolean { + const path = run.replace(/[\\/]+/g, "/").replace(/[/.]+$/, "").toLowerCase(); + if (DOT_DOT.test(path)) return false; + return EXACT.has(path) || SUBTREES.some((root) => path === root || path.startsWith(`${root}/`)); +} + +/** The text as the detector reads it: NFC, no format characters, decoded separators, collapsed runs, flags apart. */ +const prepare = (text: string) => + collapseSeparators(decodeSeparators(unicodeSlashes(text.replace(/\p{Cf}/gu, "").normalize("NFC")))).replace(GLUED_FLAG, "$1 "); + +/** True when `text` names an absolute local path. */ +export function hasLocalPath(text: string): boolean { + const decoded = prepare(text); + // Still encoded after the last round: the path cannot be read, so it is refused. + if (ENCODED.test(decoded) || LOCAL_PATHS.some((re) => re.test(decoded))) return true; + const plain = decoded.replace(WEB_URL, " "); + let runEnd = -1; + for (const m of plain.matchAll(POSIX_PATH)) { + const start = m.index!; + // Each path run is judged once, as a whole, from its first match. + if (start < runEnd) continue; + PATH_RUN.lastIndex = start; + PATH_RUN.exec(plain); + runEnd = PATH_RUN.lastIndex; + if (!allowedSystemPath(plain.slice(start, runEnd))) return true; + } + return false; +} diff --git a/apps/web/tests/personal-kits/kit-proposal-activation.test.ts b/apps/web/tests/personal-kits/kit-proposal-activation.test.ts new file mode 100644 index 0000000..aa58a6a --- /dev/null +++ b/apps/web/tests/personal-kits/kit-proposal-activation.test.ts @@ -0,0 +1,179 @@ +/** + * Activation gate of session-derived content. A revision with a + * session-derived revision anywhere in its history, whatever its own origin + * (manual, rebase, resolution), also through the revision of a parent kit + * its document names, or with a parent that cannot be read, + * activates (and rolls back) only from a web session and only as a private + * document on a private kit. Any activation that makes a kit public needs a + * web session too; other private activations keep working with an API key. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { BODY, createKitHarness, skillText } from "./kit-harness"; +import type { KitHarness } from "./kit-harness"; + +const session = vi.hoisted(() => ({ user: null as { id: string } | null })); +vi.mock("~/lib/auth/session-helpers", () => ({ + getSession: async () => (session.user === null ? null : { user: session.user }), +})); + +type Rev = { id: string; digest: string }; +let k: KitHarness; +let manual: Rev; +const WEB = { key: null }; +const MINE = { "SKILL.md": skillText(BODY.replace("two\n", "two (mine)\n")), "notes.md": "my notes\n" }; +const LEARNED = { ...MINE, "notes.md": "my notes\nprefer small steps\n" }; +const v2 = (from: string, to: string) => ({ "SKILL.md": skillText(BODY.replace(from, to)) }); +const gated = (res: { status: number; body: Record }) => { + expect(res.status).toBe(403); + expect(res.body.error.code).toBe("kit.interactive_approval_required"); +}; + +beforeEach(async () => { + session.user = { id: "user-a" }; + k = await createKitHarness(); + await k.publish("@alpha/demo", "1.0.0", { "SKILL.md": skillText(BODY) }); + manual = (await k.call("revisions", { expectedGeneration: 0, idempotencyKey: k.idem(), document: k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)]) })).body.revision; +}); + +/** A reviewed session-derived revision of the kit; returns it and the kit generation after the review. */ +async function reviewedProposal(): Promise<{ rev: Rev; generation: number }> { + const e = await k.entry("@alpha/demo", "1.0.0", LEARNED); + const proposal = { schema: "skillx.kit-proposal/v1", fromRevisionId: manual.id, skill: { identity: e.identity, base: e.base, overlay: e.overlay, compositionDigest: e.compositionDigest }, declaredPermissions: [], visibility: "private" }; + const rev = (await k.call("proposals", { expectedGeneration: 1, idempotencyKey: k.idem(), proposal })).body.revision as Rev; + const review = await k.call(`revisions/${rev.id}/review`, { expectedGeneration: 2, decision: "approve", approval: { revisionDigest: rev.digest, permissions: [], visibility: "private", consent: true } }, WEB); + expect(review.status).toBe(200); + return { rev, generation: review.body.generation }; +} + +const activation = (rev: Rev, generation: number) => ({ expectedGeneration: generation, revisionId: rev.id, approval: { revisionDigest: rev.digest } }); + +/** The reviewed proposal, made effective from a web session; returns the generation after it. */ +async function effectiveProposal(): Promise<{ rev: Rev; generation: number }> { + const { rev, generation } = await reviewedProposal(); + const act = await k.call("activate", activation(rev, generation), WEB); + expect(act.status).toBe(200); + return { rev, generation: act.body.generation }; +} + +/** A manual revision; its parent is the effective revision. */ +async function manualRevision(generation: number, note: string, visibility = "private") { + const document = k.doc([await k.entry("@alpha/demo", "1.0.0", { ...LEARNED, "notes.md": note })], { visibility }); + const res = await k.call("revisions", { expectedGeneration: generation, idempotencyKey: k.idem(), document }); + expect(res.status).toBe(201); + return { rev: res.body.revision as Rev & { parentRevisionId: string }, generation: res.body.generation as number }; +} + +const rebase = (fromRevisionId: string, version: string, expectedGeneration: number) => + k.call("rebase", { expectedGeneration, idempotencyKey: k.idem(), fromRevisionId, updates: [{ package: "@alpha/demo", version }] }); + +/** Activation by API key is refused with the interactive gate; a web session then activates. */ +async function expectWebOnly(rev: Rev, generation: number, action = "activate") { + gated(await k.call(action, activation(rev, generation))); + const byWeb = await k.call(action, activation(rev, generation), WEB); + expect(byWeb.status).toBe(200); + const stored = k.h.d1.raw.prepare("SELECT approval_json FROM personal_kit_revisions WHERE id = ?").get(rev.id) as { approval_json: string }; + expect(JSON.parse(stored.approval_json)).toMatchObject({ credential: "web-session" }); + return byWeb.body.generation as number; +} + +describe("revisions derived from a proposal", () => { + it("gates a rebase draft of a reviewed proposal, also through a chain of rebases", async () => { + const { rev, generation } = await reviewedProposal(); + await k.publish("@alpha/demo", "2.0.0", v2("eight\n", "eight (v2)\n")); + const first = await rebase(rev.id, "2.0.0", generation); + expect(first.body.revision).toMatchObject({ state: "draft", origin: "rebase", parentRevisionId: rev.id }); + await k.publish("@alpha/demo", "3.0.0", v2("seven\n", "seven (v3)\n")); + const second = await rebase(first.body.revision.id, "3.0.0", first.body.generation); + expect(second.body.revision).toMatchObject({ origin: "rebase", parentRevisionId: first.body.revision.id }); + await expectWebOnly(second.body.revision, second.body.generation); + }); + + it("gates a conflict resolution of a reviewed proposal", async () => { + const { rev, generation } = await reviewedProposal(); + await k.publish("@alpha/demo", "2.0.0", v2("two\n", "two (v2)\n")); + const clash = await rebase(rev.id, "2.0.0", generation); + expect(clash.body.status).toBe("needs-review"); + const merged = await k.overlayTo("@alpha/demo", "2.0.0", { ...LEARNED, "SKILL.md": skillText(BODY.replace("two\n", "two (mine, v2)\n")) }); + const res = await k.call(`conflicts/${clash.body.conflictId}/resolve`, { + expectedGeneration: clash.body.generation, + idempotencyKey: k.idem(), + approvedBy: "user", + decisions: [{ identity: "@alpha/demo", choice: "edit-overlay", ...merged }], + }); + expect(res.status).toBe(201); + expect(res.body.revision).toMatchObject({ origin: "resolution", parentRevisionId: rev.id }); + await expectWebOnly(res.body.revision, res.body.generation); + }); + + it("gates a manual revision stored over an effective proposal, and a rebase of it", async () => { + const { rev, generation } = await effectiveProposal(); + const next = await manualRevision(generation, "edited by hand\n"); + expect(next.rev).toMatchObject({ origin: "manual", parentRevisionId: rev.id }); + const after = await expectWebOnly(next.rev, next.generation); + await k.publish("@alpha/demo", "2.0.0", v2("eight\n", "eight (v2)\n")); + const rebased = await rebase(next.rev.id, "2.0.0", after); + expect(rebased.body.revision).toMatchObject({ origin: "rebase", parentRevisionId: next.rev.id }); + await expectWebOnly(rebased.body.revision, rebased.body.generation); + }); + + it("gates a rollback onto a session-derived revision", async () => { + const { rev, generation } = await effectiveProposal(); + const next = await manualRevision(generation, "edited by hand\n"); + const after = await expectWebOnly(next.rev, next.generation); + await expectWebOnly(rev, after, "rollback"); + }); + + it("keeps a revision derived from a proposal private, even from a web session", async () => { + const { generation } = await effectiveProposal(); + const pub = await manualRevision(generation, "published\n", "public"); + const act = await k.call("activate", activation(pub.rev, pub.generation), WEB); + expect(act.status).toBe(409); + expect(act.body.error.code).toBe("proposal.private_kit_required"); + }); + + it("gates a revision whose parent cannot be read (fails closed)", async () => { + await k.publish("@alpha/demo", "2.0.0", v2("eight\n", "eight (v2)\n")); + const res = await rebase(manual.id, "2.0.0", 1); + expect(res.status).toBe(201); + k.sql("DROP TRIGGER personal_kit_revisions_immutable"); + k.sql("UPDATE personal_kit_revisions SET parent_revision_id = ? WHERE id = ?", "pkr_00000000-0000-0000-0000-000000000000", manual.id); + gated(await k.call("activate", activation(res.body.revision, res.body.generation))); + }); + + it("gates a kit whose document parent is a session-derived revision of another kit", async () => { + const { rev } = await reviewedProposal(); + const CHILD = { kit: "@alpha/child" }; + const document = k.doc([], { kit: CHILD.kit, localId: "local/child", parent: { kit: "@alpha/my-kit", revisionDigest: rev.digest } }); + const stored = await k.call("revisions", { expectedGeneration: 0, idempotencyKey: k.idem(), document }, CHILD); + expect(stored.status).toBe(201); + const child = stored.body.revision as Rev; + gated(await k.call("activate", activation(child, 1), CHILD)); + const byWeb = await k.call("activate", activation(child, 1), { ...WEB, ...CHILD }); + expect(byWeb.status).toBe(200); + }); +}); + +describe("activations without session-derived content", () => { + it("keeps API-key activation for private drafts that descend from manual revisions", async () => { + await k.publish("@alpha/demo", "2.0.0", v2("eight\n", "eight (v2)\n")); + const res = await rebase(manual.id, "2.0.0", 1); + expect(res.status).toBe(201); + const act = await k.call("activate", activation(res.body.revision, res.body.generation)); + expect(act.status).toBe(200); + }); + + it("makes a kit public only from a web session", async () => { + const pub = await manualRevision(1, "published\n", "public"); + await expectWebOnly(pub.rev, pub.generation); + expect((await k.call("")).body.visibility).toBe("public"); + }); + + it("creates every kit private, also from a public first document", async () => { + const CHILD = { kit: "@alpha/child" }; + const document = k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)], { kit: CHILD.kit, localId: "local/child", visibility: "public" }); + const res = await k.call("revisions", { expectedGeneration: 0, idempotencyKey: k.idem(), document }, CHILD); + expect(res.status).toBe(201); + expect((await k.call("", undefined, CHILD)).body.visibility).toBe("private"); + gated(await k.call("activate", activation(res.body.revision, 1), CHILD)); + }); +}); diff --git a/apps/web/tests/personal-kits/kit-proposal-safety.test.ts b/apps/web/tests/personal-kits/kit-proposal-safety.test.ts index 7f17049..5762e94 100644 --- a/apps/web/tests/personal-kits/kit-proposal-safety.test.ts +++ b/apps/web/tests/personal-kits/kit-proposal-safety.test.ts @@ -166,7 +166,7 @@ describe("proposal approval", () => { const rev = (await submit(await proposal(with_("x\n")), 1)).body.revision; await approve(rev, 2); const pub = (await k.call("revisions", { expectedGeneration: 3, idempotencyKey: k.idem(), document: k.doc([await k.entry("@alpha/demo", "1.0.0", MINE)], { visibility: "public" }) })).body.revision; - expect((await k.call("activate", { expectedGeneration: 4, revisionId: pub.id, approval: { revisionDigest: pub.digest } })).status).toBe(200); + expect((await k.call("activate", { expectedGeneration: 4, revisionId: pub.id, approval: { revisionDigest: pub.digest } }, WEB)).status).toBe(200); const act = await k.call("activate", { expectedGeneration: 5, revisionId: rev.id, approval: { revisionDigest: rev.digest } }, WEB); expect(act.status).toBe(409); expect(act.body.error.code).toBe("proposal.private_kit_required"); diff --git a/apps/web/tests/personal-kits/kit-safety.test.ts b/apps/web/tests/personal-kits/kit-safety.test.ts index 145a528..1e912e1 100644 --- a/apps/web/tests/personal-kits/kit-safety.test.ts +++ b/apps/web/tests/personal-kits/kit-safety.test.ts @@ -9,7 +9,8 @@ import { ReleaseHttpError } from "~/lib/releases/release-http"; import { BODY, createKitHarness, skillText } from "./kit-harness"; import type { KitHarness } from "./kit-harness"; -vi.mock("~/lib/auth/session-helpers", () => ({ getSession: async () => null })); +const session = vi.hoisted(() => ({ user: null as { id: string } | null })); +vi.mock("~/lib/auth/session-helpers", () => ({ getSession: async () => (session.user === null ? null : { user: session.user }) })); let k: KitHarness; const V1 = { "SKILL.md": skillText(BODY) }; @@ -22,14 +23,16 @@ beforeEach(async () => { await k.publish("@alpha/demo", "1.0.0", V1); }); -/** A revision of user-b's kit `@beta/base`; activated when `activate` is set. */ +/** A revision of user-b's kit `@beta/base`; activated when `activate` is set (in user-b's web session: a public activation needs one). */ async function betaRevision(generation: number, visibility: "private" | "public", activate: boolean, files = MINE) { const document = { ...k.doc([await k.entry("@alpha/demo", "1.0.0", files)], { visibility }), kit: "@beta/base", localId: "local/base" }; const res = await k.call("revisions", { expectedGeneration: generation, idempotencyKey: k.idem(), document }, { ...BETA, key: k.otherKey }); expect(res.status).toBe(201); const rev = res.body.revision as { id: string; digest: string }; if (activate) { - const ok = await k.call("activate", { expectedGeneration: generation + 1, revisionId: rev.id, approval: { revisionDigest: rev.digest } }, { ...BETA, key: k.otherKey }); + session.user = { id: "user-b" }; + const ok = await k.call("activate", { expectedGeneration: generation + 1, revisionId: rev.id, approval: { revisionDigest: rev.digest } }, { ...BETA, key: null }); + session.user = null; expect(ok.status).toBe(200); } return rev; diff --git a/apps/web/tests/personal-kits/proposal-detector-cost.test.ts b/apps/web/tests/personal-kits/proposal-detector-cost.test.ts new file mode 100644 index 0000000..9452ec4 --- /dev/null +++ b/apps/web/tests/personal-kits/proposal-detector-cost.test.ts @@ -0,0 +1,87 @@ +/** + * The proposal detectors run in time linear in the text. Each input is built + * to make a backtracking pattern quadratic (long separator runs, `a-a-a-...` + * scheme runs, `eyJ-eyJ-...`, repeated keys, blank lines under a YAML key) + * and scanned at 128 KiB and at 1 MiB: 8 times the text may cost at most 24 + * times the time (a quadratic pattern costs 64 times), measured as the best + * of several runs, with a small floor so timer noise on fast inputs is not + * read as growth. No wall-clock bound, so a slow runner does not fail it. + */ +import { describe, expect, it } from "vitest"; +import { genericSecrets, hasLocalPath, hasMultilinePassword } from "~/lib/personal-kits/proposal-detectors"; + +const SIZE = 1024 * 1024; +const SMALL = SIZE / 8; +const repeatTo = (unit: string, size: number) => unit.repeat(Math.ceil(size / unit.length)).slice(0, size); +const fill = (unit: string) => repeatTo(unit, SIZE); +const MAX_RATIO = 24; +const FLOOR_MS = 5; +const RUNS = 3; + +type Build = (size: number) => string; +const unit = (u: string): Build => (size) => repeatTo(u, size); +const after = (head: string, u: string): Build => (size) => head + repeatTo(u, size); + +const HOSTILE: Record = { + "slashes": unit("/"), + "backslashes": unit("\\"), + "JSON-escaped slashes": unit("\\/"), + "percent-encoded slashes": unit("%2F"), + "double-encoded slashes": unit("%252F"), + "scheme-like dashes": unit("a-"), + "scheme-like pluses": unit("a+"), + "drive-like colons": unit("a:"), + "JWT-like heads": unit("eyJ-"), + "dollar segments": unit("/usr/$"), + "allowed system paths": unit(" /usr/bin/x"), + "climbing segments": unit("/a/.."), + "editor links": unit("x://file"), + "glued flags": unit(" -I"), + "flag-like dashes": after(" -", "a"), + "one long segment": after(" /", "a"), + "non-ASCII segments": unit("/é"), + "UNC-like runs": unit("\\\\ab"), + "secret key words": unit("TOKEN"), + "password words": unit("password"), + "password assignments": unit("password: "), + "chained assignments": unit("a="), + "flag-like assignments": unit("-a-a-a="), + "quoted key chains": unit('"k":'), + "long keys": unit("k".repeat(128) + "="), + "long secret values": unit("token=" + "v".repeat(300) + ";"), + "anchors and tags": unit("password: &a !s "), + "encoded dot layers": unit("%25"), + "Unicode slashes": unit("\uff0fa"), + "credential URLs": unit("x://a:"), + "YAML password keys": unit("password:\n"), + "YAML list password keys": unit("- password:\n \n"), + "private key heads": unit("-----BEGIN A "), + "blank lines under a YAML password key": (size) => "password:\n" + repeatTo(" \n", size) + "x\n", +}; + +describe("detector cost on hostile input", () => { + /** The best of `RUNS` timings of every detector on `text`. */ + function cost(text: string): number { + let best = Infinity; + for (let i = 0; i < RUNS; i++) { + const started = performance.now(); + hasLocalPath(text); + genericSecrets(text); + hasMultilinePassword(text, text); + best = Math.min(best, performance.now() - started); + } + return best; + } + + it.each(Object.entries(HOSTILE))("scans %s in linear time", (_name, build) => { + const small = cost(build(SMALL)); + const large = cost(build(SIZE)); + expect(large).toBeLessThan(MAX_RATIO * Math.max(small, FLOOR_MS)); + }); + + it("still finds a path or secret at the end of a hostile run", () => { + expect(hasLocalPath(fill("/") + "home/kai")).toBe(true); + expect(hasLocalPath(fill("a-") + " /srv/x/y")).toBe(true); + expect(genericSecrets(fill("eyJ-") + " " + ["eyJ", "hbGciOiJIUzI1NiJ9", ".eyJ", "zdWIiOiIxIn0", ".c2ln"].join(""))).toEqual(["secret-jwt"]); + }); +}); diff --git a/apps/web/tests/personal-kits/proposal-detectors.test.ts b/apps/web/tests/personal-kits/proposal-detectors.test.ts index 3b0fcbc..f4fe39a 100644 --- a/apps/web/tests/personal-kits/proposal-detectors.test.ts +++ b/apps/web/tests/personal-kits/proposal-detectors.test.ts @@ -1,130 +1,52 @@ import { describe, expect, it } from "vitest"; +import vectors from "../../../../packages/contracts/src/fixtures/kit-proposal-policy-vectors-v1.json"; import { addedLines, genericSecrets, hasByteOrderMark, hasLocalPath, hasMultilinePassword } from "~/lib/personal-kits/proposal-detectors"; import { sharedSkills } from "~/lib/personal-kits/kit-verify"; import { assertNoForbiddenKeys } from "~/lib/personal-kits/proposal-envelope"; -describe("absolute local path detector", () => { - it.each([ - String.raw`"cwd": "C:\\Users\\kai\\repo"`, - String.raw`D:\www\project\notes.md`, - "d:/data/file", - String.raw`share \\srv01\public\x`, - String.raw`"\\\\srv01\\share\\x"`, - "file:///home/kai/x", - "file://host/share", - "/mnt/c/Users/kai", - ">/home/kai/log", - " expect(hasLocalPath(text)).toBe(true)); +/** The shared `skillx.kit-proposal-policy/v1` content vectors (AgentKit tests against the same file). */ +const joined = (v: { parts: string[] }) => v.parts.join(""); - it.each([ - "#!/usr/bin/env node", - "https://example.com/Users/guide", - "src/tmp/file", - "./tmp/x", - "~/notes", - "docs/home/page", - "at 12:30 today", - "http://localhost:3000/api", - String.raw`"\\n\\t"`, - "ratio a:b", - "/homepage and /tmpl", - // The system allowlist, in any case. - "#!/bin/sh", - "/usr/local/bin/tool", - "/sbin/ifconfig", - "edit /etc/hosts", - "/proc/self/status", - "cmd 2>/dev/null", - "read /dev/stdin, write /dev/stdout and /dev/stderr", - "/USR/BIN/env", - "https://example.com/var/www/x?y=%2Fz", - "dates 12/31/2025 and 1/2", - "run /ak:plan now", - ])("allows %s", (text) => expect(hasLocalPath(text)).toBe(false)); +describe("absolute local path detector", () => { + it.each(vectors.localPaths.refuse)("refuses %s", (text) => expect(hasLocalPath(text)).toBe(true)); + it.each(vectors.localPaths.allow)("allows %s", (text) => expect(hasLocalPath(text)).toBe(false)); }); describe("generic secret detector", () => { - // Built at runtime so no credential-shaped literal sits in the source. - const jwt = ["eyJ", "hbGciOiJIUzI1NiJ9", ".eyJ", "zdWIiOiIxIn0", ".c2ln"].join(""); - it.each([ - ["password=" + "hunter22", "secret-password"], - ["db_passwd: " + "hunter22", "secret-password"], - ["Authorization: Bearer " + "a1".repeat(12), "secret-bearer"], - ["postgres://admin:" + "s3cretpw" + "@db.internal/x", "secret-url-credentials"], - ["token " + jwt, "secret-jwt"], - [["-----BEGIN ", "OPENSSH PRIVATE KEY-----"].join(""), "secret-private-key"], - ["OPENAI_API_KEY=" + "x9".repeat(10), "secret-dotenv"], - ["export DB_PASSWORD=" + "q".repeat(12), "secret-dotenv"], - ])("finds %s", (text, rule) => expect(genericSecrets(text)).toContain(rule)); - - it.each(["password: ", "password=${PASSWORD}", "Bearer ", "https://example.com/a@b", "API_KEY=", "the password field", "AUTHOR_NAME=Jonathan"])( - "ignores %s", - (text) => expect(genericSecrets(text)).toEqual([]), - ); + it.each(vectors.secrets.map((v) => [joined(v), v.rules] as const))("finds in %j exactly %j", (text, rules) => expect(genericSecrets(text)).toEqual(rules)); }); describe("multi-line YAML password detector", () => { - const value = "hunter" + "22"; - it.each([ - `db:\n password:\n ${value}\n`, - `password: |\n ${value}\n`, - `password: >-\n ${value}\n more\n`, - `PASSWORD:\n\n ${value}\n`, - ])("finds %j", (text) => expect(hasMultilinePassword(null, text)).toBe(true)); - - it.each(["password: |\n \n", "password:\n ${DB_PASSWORD}\n", "password:\nnext: 1\n", "password: |\n ab\n"])( - "ignores %j", - (text) => expect(hasMultilinePassword(null, text)).toBe(false), - ); + it.each(vectors.multilinePasswords.refuse.map(joined))("finds %j", (text) => expect(hasMultilinePassword(null, text)).toBe(true)); + it.each(vectors.multilinePasswords.allow.map(joined))("ignores %j", (text) => expect(hasMultilinePassword(null, text)).toBe(false)); it("finds a new value under a key the base already has, not a block kept from the base", () => { + const value = "hunter" + "22"; const base = "db:\n password:\n \n"; expect(hasMultilinePassword(base, `db:\n password:\n ${value}\n`)).toBe(true); const kept = `db:\n password:\n ${value}\n`; expect(hasMultilinePassword(kept, `${kept}other: 1\n`)).toBe(false); + const list = `users:\n - password:\n ${value}\n`; + expect(hasMultilinePassword(list, `${list} - name: b\n`)).toBe(false); + }); +}); + +describe("forbidden keys through the envelope", () => { + it.each(vectors.keys.map((v) => [v.name, v] as const))("%s", (_name, v) => { + if (v.refusedAt === null) return expect(() => assertNoForbiddenKeys(v.value)).not.toThrow(); + expect(() => assertNoForbiddenKeys(v.value)).toThrow(`${v.refusedAt} is not accepted`); + }); + + it("never echoes the value", () => { + const value = "hunter" + "22"; + let message = ""; + try { + assertNoForbiddenKeys({ evidence: { password: value } }); + } catch (error) { + message = (error as Error).message; + } + expect(message).toContain("proposal.evidence.password"); + expect(message).not.toContain(value); }); }); diff --git a/docs/personal-kits-api.md b/docs/personal-kits-api.md index c326530..5f79a70 100644 --- a/docs/personal-kits-api.md +++ b/docs/personal-kits-api.md @@ -90,7 +90,10 @@ without an explicit user approval. `ak:` (any case) is always shared, whether or not `SKILLX_SHARED_SKILL_ALIASES` is set. That variable (comma or whitespace separated identities) adds further shared skills to this default. Other - `@agentkit/*` skills may be overlaid. + `@agentkit/*` skills may be overlaid. AgentKit kit composition owns the + canonical list: the non-`ak-` skills that `kits/core` or more than one + official kit ships. Operators set the variable from that list; SkillX never + derives it. - A fork alias that starts with `ak-` or `ak:` is refused with 422 `identity.reserved_alias` (AgentKit code). - Kit names, identities and fork identities pass the full identity rules @@ -152,11 +155,11 @@ Every kit has a `generation`. `GET` returns it, with an `ETag` of `"g"`. |---|---| | `GET` | Kit summary: `kit`, `localId`, `visibility`, `generation`, `latestRevisionNumber`, `effectiveRevision`, `openConflicts`. | | `GET /revisions`, `GET /revisions/:id` | Revision list, or one revision with its `document`. | -| `POST /revisions` `{expectedGeneration, idempotencyKey, document}` | Store a `draft` revision with origin `manual`. 201 `{revision, generation, replayed, warnings}`. The first write creates the kit. | +| `POST /revisions` `{expectedGeneration, idempotencyKey, document}` | Store a `draft` revision with origin `manual`. 201 `{revision, generation, replayed, warnings}`. The first write creates the kit, always `private`: only an activation sets the kit's visibility, so an API key never creates a public kit. | | `POST /revisions/:id/reject` `{expectedGeneration}` | Reject a draft. Rejecting the effective revision fails with 409 `kit.revision_effective`. A session-derived proposal is refused with 403 `kit.interactive_approval_required`; it is rejected only through `POST /revisions/:id/review` in a web session. | | `GET /diff?from=&to=` | Read-only structural diff: changed document fields, and added, removed and changed entries with their overlay paths. | | `POST /preview-update` `{updates:[{package,version}], fromRevisionId?, includeForks?}` | Rebase report only. Writes nothing. | -| `POST /rebase` `{expectedGeneration, idempotencyKey, updates, fromRevisionId?, includeForks?}` | Clean: 201, a `draft` revision with origin `rebase` (never activated). Otherwise: 200 `{status:"needs-review", conflictId, report}`. A newer conflict supersedes older open ones. | +| `POST /rebase` `{expectedGeneration, idempotencyKey, updates, fromRevisionId?, includeForks?}` | Clean: 201, a `draft` revision with origin `rebase` (never activated implicitly). Otherwise: 200 `{status:"needs-review", conflictId, report}`. A newer conflict supersedes older open ones. | | `GET /conflicts`, `GET /conflicts/:id` | Conflict records: the report, the resolution and `reevalRequired`. | | `POST /conflicts/:id/resolve` `{expectedGeneration, idempotencyKey, approvedBy:"user", decisions, agentSuggestion?}` | The user's decision becomes a `draft` revision with origin `resolution`. | | `POST /activate`, `POST /rollback` `{expectedGeneration, revisionId, approval:{revisionDigest}}` | Make a revision effective. `rollback` re-activates an earlier effective revision. | @@ -229,7 +232,10 @@ written. ### Activation, rollback and lock Activation requires `approval.revisionDigest` to equal the exact revision -digest, otherwise 409 `kit.approval_required`. Before moving +digest, otherwise 409 `kit.approval_required`. Activating (or rolling back +to) a revision whose document is `public` needs an interactive web session, +whatever its origin: an API key gets 403 `kit.interactive_approval_required`, +so a kit becomes public only by a signed-in owner. Before moving `effective_revision_id`, it re-checks every pinned base of the lineage: - A base that is quarantined, revoked, no longer entitled or removed blocks @@ -258,13 +264,32 @@ proposal is always private. The owner reviews it, then activates it as usual. Nothing activates implicitly. - **Private kits only**: a proposal for a public kit fails with 409 - `proposal.private_kit_required` (fork it to a private kit first), and so - does activating a session-derived revision once the kit has become public. - The kit's visibility is never flipped by a proposal. + `proposal.private_kit_required` (fork it to a private kit first). So does + activating a gated revision (below) while the kit is public, or when its + own document is `public`: the check covers the visibility the kit would + have after activation, not only its current one. The kit's visibility is + never flipped by a proposal. - **A human approves**: review (approve or reject) and activation of a session-derived revision need an interactive web session. An API key, such as the one AgentKit submits with, gets 403 `kit.interactive_approval_required`. The proofs record `credential`. +- **The gate is inherited**: a revision is gated when it, or any ancestor in + its revision chain, is session-derived, whatever the origins in between + (manual, rebase, resolution). The same holds for the revision a document's + `parent` names in another kit, and for that revision's own chain, at every + level of the lineage. A parent that cannot be found or read counts as + gated, or fails the activation with `kit.locked_incompatible`. A gated + revision activates (or is rolled back to) only from a web session, only on + a private kit, and only as a private document. +- **What the gate follows**: the gate follows revision history, not + content. A manual revision's parent is the kit's *effective* revision, so + once a session-derived revision has been effective, the revisions built on + it stay gated. A session-derived revision that never became effective + gates nothing but its own rebases and resolutions and the kits whose document `parent` names it. Rolling back to an + earlier private revision with no session-derived ancestor (allowed with an + API key) ends the gate for the revisions built after it. Content copied + into a revision by hand carries no history, so it is not gated; making it + public still needs a web session. - **No chaining on unreviewed proposals**: a from-revision that is `proposed` (or any session-derived revision that is not `reviewed`) fails with 409 `kit.review_required`, and an unreviewed proposal is never usable as a @@ -310,8 +335,26 @@ verifies the result like any revision. messages, conversation, environment, secret or token, working directory, history, tool output or logs, at any depth, fails with 422 `proposal.forbidden_field`. The message names the key path, never its value. - A key with a numeric value (an evidence signal) is allowed. Unknown fields - are refused. An unknown top-level body field is 400 `invalid_request`. + A path is `proposal`, then `.key` for a key of `[A-Za-z0-9_$-]`, `["key"]` + (a JSON string) for any other key, so `.` and `[` in a key are never + ambiguous, `[index]` for an array item, `#json` for a JSON string read as + nested keys, and `[…]` for a key over the length limit, which is never + echoed. Keys are compared in this order: refused when longer than 64 code + points; percent-decoded up to 3 rounds (refused when an escape remains); + refused unless every character is printable ASCII or its fullwidth form + (U+FF01 to U+FF5E); NFKC (Unicode 15.0, pinned in the policy; only the + fullwidth forms change, so the result does not depend on the runtime's + Unicode version); refused unless printable ASCII; `A-Z` lowercased; + everything but `a-z0-9` removed (`Session_Log`, `api-key`, `%70assword`, + fullwidth letters). A string that holds a JSON object or array, also + inside JSON string layers (outside `contentBase64` and `patch`), is + checked as nested keys. A key with + a numeric value (an evidence signal) is allowed. Unknown fields are refused. + An unknown top-level body field is 400 `invalid_request`. The key lists and + the content rules are the shared policy `skillx.kit-proposal-policy/v1` in + `packages/contracts/src/fixtures/kit-proposal-policy-v1.json`; AgentKit + applies the same file before upload, and both sides test against + `kit-proposal-policy-vectors-v1.json` next to it. - **Visibility**: anything but `"private"` fails with 422 `proposal.private_only`. - **Evidence**: optional. `summary` is at most 2048 UTF-8 bytes @@ -321,24 +364,56 @@ verifies the result like any revision. composition (the lines a changed file gains, a new file whole), plus the evidence summary as `evidence/summary.md`. Lines kept from the base are not scanned, so a finding already in the base never refuses a proposal. - - Fails with 422 `proposal.content_refused`, nothing stored: a secret - (vendor token formats, `password=`/`passwd:` assignments, a YAML - `password:` key whose new value sits on the following indented lines or in - a `|` / `>` block scalar, even when the key line is kept from the base, - Bearer tokens, URLs with `user:pass@`, JWTs, private key blocks, - dotenv-style secret keys), binary content, text with a byte order mark - (UTF-8/16/32), and every absolute local path, matched case-insensitively - and also JSON-escaped (`\/`, `\\`) or percent-encoded (`%2F`, `%5C`, - `%3A`): any drive-letter path, UNC shares (`\\host\share` and - `//host/share`), `file:` and `smb:` URLs, a bare `/home`, `/Users`, - `/root`, `/mnt`, `/media`, `/Volumes`, `/var/folders`, `/tmp`, `/private` - or `/run/user`, and any other POSIX path of two or more segments (such as - `/opt/...`, `/srv/...`, `/var/www/...`, `/workspaces/...`). Only these - system paths are allowed: `/usr/`, `/bin/`, `/sbin/`, `/etc/`, `/proc/`, - `/dev/null`, `/dev/stdin`, `/dev/stdout` and `/dev/stderr` (a `..` segment - that climbs out of them is refused). Web URLs (`https://host/path`) are not - local paths. `error.findings` lists `danger::` codes, never - the matched text. + - Fails with 422 `proposal.content_refused`, nothing stored: a secret, + binary content, text with a byte order mark (UTF-8/16/32), or an + absolute local path. + - Secrets: vendor token formats; a `key: value`, `key = value` or + `"key": "value"` assignment, also with `:=` or `=>`, in brackets + (`cfg["password"] = ...`), as a flag (`--password=...`) or inside another + value (`Server=db;Password=...`, `?user=a&password=...`), in YAML, JSON, + `.env` or code, whose key holds a stem followed by nothing or by at most + two qualifiers of `content.secretKeys.qualifiers` or a version number + (`secret_key_base`, `api_key_prod`, `private_key_id`; not `secretName`, + `passwordless` or `tokenizer`): a password stem (`secret-password`: + `password`, `dbPassword`, `DB_PASSWD`, a value of 4 or more characters) + or a secret stem (`secret-assignment`: `clientSecret`, `apiKey`, + `accessToken`, `awsCredentials`, a value of 8 or more characters), in any case and + separator style, compared through the same key normalization as the + envelope (the stems are `content.secretKeys` in the policy); a + `KEY=value` line whose key holds a stem anywhere (`secret-dotenv`); a + YAML `password:` key whose new value sits on the following indented lines + or in a `|` / `>` block scalar, also as a list item `- password:`, a + quoted key `"password":`, or after up to two anchors or tags (`&pw`, + `!secret`, `&a !s`), + even when the key line is kept from the base; Bearer tokens, URLs with + `user:pass@`, JWTs and private key blocks. A YAML anchor or tag before an + inline value is skipped (up to two). Placeholders (`<...>`, `%VAR%`, `{...}`, `***`, + `${...}`, a whole `$VAR`) and numbers are not secrets; any other value, + also one that starts with `$`, is. + - Paths: every absolute local path, matched case-insensitively, after NFC + normalization, with Unicode slash forms (`／`, `∕`, `⁄`, `＼`) read as + separators and invisible format characters (zero-width spaces and + joiners) removed, and also JSON-escaped (`\/`, `\\`) or percent-encoded + (`%2F`, `%5C`, `%3A`, `%2E`, decoded up to three `%25` layers; a text + still holding an encoded separator or dot after that is refused): any drive-letter path, UNC + shares (`\\host\share` and `//host/share`), `file:` and `smb:` URLs, + editor links to a file (`vscode://file/...`), a user home in an `ssh` or + `sftp` URL, a bare `/home`, `/Users`, `/root`, `/mnt`, `/media`, + `/Volumes`, `/var/folders`, `/tmp`, `/private` or `/run/user`, a path + glued to a flag (`-I/opt/x`, `--include=/opt/x`), and any other POSIX + path of two or more segments in any script (such as `/opt/...`, + `/srv/...`, `/var/www/...`, `/usr/src/...`, `/proc/self/root/...`, + `/Usuários/...`). Only the policy's `localPathSystemAllowlist` is allowed: + whole `subtrees` of binary and library directories (`/usr/bin/...`, + `/usr/lib/...`, `/usr/local/bin/...`), and `exact` well-known + directories and files, matched as a whole (`/etc/hosts`, `/proc/cpuinfo`, + `/dev/null`, `/usr/share/zoneinfo`; not `/usr/share/doc/x` or + `/usr/local/opt/x`). A `..` segment refuses the path. URLs of network + schemes (`https://host/path`, `s3://`, ...) are not local paths; a path + glued after a URL's `,`, `;`, `|`, `&`, `(`, `)`, `[`, `]`, `{` or `}` is still read. + `error.findings` lists `danger::` codes, never the matched + text. Every detector runs in time linear in its input, so a hostile + request body cannot exhaust the Worker CPU. - Any other `danger` finding, such as prompt injection or a `javascript:` URL, stores the proposal as `quarantined`. A quarantined proposal cannot be reviewed or activated. diff --git a/packages/contracts/src/fixtures/kit-proposal-policy-v1.json b/packages/contracts/src/fixtures/kit-proposal-policy-v1.json new file mode 100644 index 0000000..dac698e --- /dev/null +++ b/packages/contracts/src/fixtures/kit-proposal-policy-v1.json @@ -0,0 +1,188 @@ +{ + "schema": "skillx.kit-proposal-policy/v1", + "envelope": "skillx.kit-proposal/v1", + "forbiddenKeys": { + "description": "A key anywhere in a proposal is normalized by the steps below, in order, then refused when it equals an exact name or contains a stem. A key that a step refuses cannot be checked and is refused too. A key whose value is a number is exempt (an evidence signal such as sessions: 3 cannot carry session content). String values outside the file-content fields that parse as a JSON object or array, also through up to percentDecodeRounds layers of JSON strings, are checked as nested documents. A refused key is reported by its path: .key for [A-Za-z0-9_$-]+ keys, otherwise [\"key\"] (the key as a JSON string), [index] for arrays, […] for a key longer than maxKeyLength (never echoed), and #json after a string parsed as JSON.", + "normalization": [ + "refuse when the key has more than maxKeyLength Unicode code points, as received", + "percent-decode %XX escapes (each to the code unit of its byte) while any remain, at most percentDecodeRounds rounds; refuse when an escape remains", + "refuse when any character is outside printable ASCII (U+0020 to U+007E) and its fullwidth forms (U+FF01 to U+FF5E)", + "Unicode NFKC (only fullwidth forms change, so the result is the same in every Unicode version since 3.2; unicodeVersion pins the reference)", + "refuse when any character is outside printable ASCII (U+0020 to U+007E)", + "lowercase A-Z only", + "remove every character outside [a-z0-9]" + ], + "unicodeVersion": "15.0", + "maxKeyLength": 64, + "percentDecodeRounds": 3, + "exact": [ + "chat", + "chats", + "cwd", + "dotenv", + "env", + "log", + "logs", + "message", + "messages", + "pwd", + "raw", + "session", + "sessions" + ], + "contains": [ + "accesskey", + "apikey", + "conversation", + "credential", + "environment", + "envvar", + "history", + "homedir", + "passphrase", + "passwd", + "password", + "privatekey", + "prompt", + "secret", + "sessionid", + "sessionlog", + "stderr", + "stdout", + "token", + "toolcall", + "tooloutput", + "toolresult", + "transcript", + "workdir" + ], + "contentFields": [ + "contentBase64", + "patch" + ], + "refusal": { + "code": "proposal.forbidden_field", + "reports": "the key path only, never the value" + } + }, + "content": { + "description": "What a change introduces (added lines of a changed file, a new file whole, the evidence summary) is refused on any rule below; findings are danger:: and never carry the matched text. Regular expressions are implementation detail; the vectors are the contract.", + "refusing": [ + "binary-content", + "byte-order-mark", + "local-path", + "secret-assignment", + "secret-bearer", + "secret-dotenv", + "secret-jwt", + "secret-password", + "secret-private-key", + "secret-url-credentials" + ], + "secretKeys": { + "description": "Keys of key: value, key = value and \"key\": value assignments in file content, normalized like forbiddenKeys. A key names a password (secret-password, value of 4 or more characters) or a secret (secret-assignment, 8 or more) when its normalized form holds a stem followed by at most two qualifiers (or by nothing): secretkeybase, apikeyprod, privatekeyid and clientsecretv2 are secret keys, secretname and tokenizer are not. A qualifier is one of qualifiers, or a number with an optional v. Separators are :, =, := and =>. On a KEY=value line (optionally after export) a key that is not a password key and holds any stem anywhere is secret-dotenv instead (8 or more). A key that cannot be normalized names a secret. Placeholder values (<...>, %VAR%, {...}, ***, ${...}, a whole $VAR) and plain numbers are not secrets.", + "password": [ + "passphrase", + "passwd", + "password", + "pwd" + ], + "secret": [ + "accesskey", + "apikey", + "credential", + "credentials", + "privatekey", + "secret", + "secretkey", + "token" + ], + "qualifiers": [ + "base", + "dev", + "hash", + "id", + "key", + "live", + "old", + "prod", + "staging", + "test", + "value" + ] + }, + "localPathSystemAllowlist": { + "description": "A POSIX path of two or more segments is allowed only when, in lower case with every separator run as one / and trailing separators and dots removed, it equals an exact entry or lies inside a subtree entry. A .. segment refuses it.", + "subtrees": [ + "/bin", + "/sbin", + "/usr/bin", + "/usr/include", + "/usr/lib", + "/usr/lib32", + "/usr/lib64", + "/usr/libexec", + "/usr/local/bin", + "/usr/local/include", + "/usr/local/lib", + "/usr/local/lib64", + "/usr/local/libexec", + "/usr/local/sbin", + "/usr/sbin" + ], + "exact": [ + "/dev/null", + "/dev/random", + "/dev/shm", + "/dev/stderr", + "/dev/stdin", + "/dev/stdout", + "/dev/tty", + "/dev/urandom", + "/dev/zero", + "/etc", + "/etc/apt/sources.list", + "/etc/crontab", + "/etc/environment", + "/etc/fstab", + "/etc/group", + "/etc/hostname", + "/etc/hosts", + "/etc/localtime", + "/etc/nginx/nginx.conf", + "/etc/os-release", + "/etc/passwd", + "/etc/profile", + "/etc/resolv.conf", + "/etc/shadow", + "/etc/shells", + "/etc/ssh/sshd_config", + "/etc/ssl/certs", + "/etc/ssl/certs/ca-certificates.crt", + "/etc/sudoers", + "/etc/timezone", + "/proc/cpuinfo", + "/proc/loadavg", + "/proc/meminfo", + "/proc/mounts", + "/proc/self/limits", + "/proc/self/status", + "/proc/stat", + "/proc/uptime", + "/proc/version", + "/usr", + "/usr/local", + "/usr/local/cellar", + "/usr/local/etc", + "/usr/local/man", + "/usr/local/opt", + "/usr/local/share", + "/usr/share", + "/usr/share/doc", + "/usr/share/fonts", + "/usr/share/man", + "/usr/share/zoneinfo" + ] + } + } +} diff --git a/packages/contracts/src/fixtures/kit-proposal-policy-vectors-v1.json b/packages/contracts/src/fixtures/kit-proposal-policy-vectors-v1.json new file mode 100644 index 0000000..b3d3ea0 --- /dev/null +++ b/packages/contracts/src/fixtures/kit-proposal-policy-vectors-v1.json @@ -0,0 +1,1250 @@ +{ + "schema": "skillx.kit-proposal-policy-vectors/v1", + "policy": "skillx.kit-proposal-policy/v1", + "description": "Shared test vectors. keys: run the forbidden-key check over value (the root path is proposal); refusedAt is the first refused key path in document order, or null. secrets.parts and multilinePasswords.*.parts are joined with no separator before use.", + "keys": [ + { + "name": "a benign envelope with every known field", + "value": { + "schema": "skillx.kit-proposal/v1", + "fromRevisionId": "pkr_00000000-0000-4000-8000-000000000000", + "skill": { + "identity": "@alpha/demo", + "base": { + "package": "@alpha/demo", + "version": "1.0.0", + "releaseDigest": "sha256:0000000000000000000000000000000000000000000000000000000000000000" + }, + "overlay": { + "schema": "agentkit.skill-overlay/v1", + "baseTreeDigest": "sha256:0000000000000000000000000000000000000000000000000000000000000000", + "ops": [ + { + "op": "patch", + "path": "SKILL.md", + "expectSha256": "0000000000000000000000000000000000000000000000000000000000000000", + "patch": "{\"prompt\": \"file content is scanned by the content rules\"}" + }, + { + "op": "add", + "path": "notes.md", + "contentBase64": "bm90ZXMK", + "mode": "100644" + } + ] + }, + "compositionDigest": "sha256:0000000000000000000000000000000000000000000000000000000000000000" + }, + "declaredPermissions": [], + "evidence": { + "summary": "prefer small steps", + "signals": { + "sessions": 3, + "tokens": 1200, + "tool_calls": 4 + } + }, + "visibility": "private" + }, + "refusedAt": null + }, + { + "name": "top-level transcript", + "value": { + "transcript": "x" + }, + "refusedAt": "proposal.transcript" + }, + { + "name": "upper case", + "value": { + "TRANSCRIPT": "x" + }, + "refusedAt": "proposal.TRANSCRIPT" + }, + { + "name": "camel case under skill", + "value": { + "skill": { + "systemPrompt": "x" + } + }, + "refusedAt": "proposal.skill.systemPrompt" + }, + { + "name": "kebab case", + "value": { + "session-log": "x" + }, + "refusedAt": "proposal.session-log" + }, + { + "name": "snake case", + "value": { + "api_key": "x" + }, + "refusedAt": "proposal.api_key" + }, + { + "name": "dotted (bracket-quoted in the path)", + "value": { + "api.key": "x" + }, + "refusedAt": "proposal[\"api.key\"]" + }, + { + "name": "spaced title case", + "value": { + "Tool Calls": [] + }, + "refusedAt": "proposal[\"Tool Calls\"]" + }, + { + "name": "brackets in a key stay unambiguous", + "value": { + "a[0]": { + "secret": "x" + } + }, + "refusedAt": "proposal[\"a[0]\"].secret" + }, + { + "name": "stem inside a longer key", + "value": { + "rawTranscript": "x" + }, + "refusedAt": "proposal.rawTranscript" + }, + { + "name": "stem with a suffix", + "value": { + "sessionLogs": [] + }, + "refusedAt": "proposal.sessionLogs" + }, + { + "name": "vendor secret name", + "value": { + "AWS_SECRET_ACCESS_KEY": "x" + }, + "refusedAt": "proposal.AWS_SECRET_ACCESS_KEY" + }, + { + "name": "password stem", + "value": { + "dbPassword": "x" + }, + "refusedAt": "proposal.dbPassword" + }, + { + "name": "history stem", + "value": { + "conversationHistory": [] + }, + "refusedAt": "proposal.conversationHistory" + }, + { + "name": "exact env", + "value": { + "evidence": { + "env": { + "HOME": "x" + } + } + }, + "refusedAt": "proposal.evidence.env" + }, + { + "name": "exact cwd in an array", + "value": { + "evidence": { + "items": [ + { + "ok": 1 + }, + { + "cwd": "x" + } + ] + } + }, + "refusedAt": "proposal.evidence.items[1].cwd" + }, + { + "name": "deeply nested", + "value": { + "a": { + "b": { + "c": { + "d": { + "e": { + "messages": [] + } + } + } + } + } + }, + "refusedAt": "proposal.a.b.c.d.e.messages" + }, + { + "name": "percent-encoded", + "value": { + "%74ranscript": "x" + }, + "refusedAt": "proposal[\"%74ranscript\"]" + }, + { + "name": "double percent-encoded", + "value": { + "%2574oken": "x" + }, + "refusedAt": "proposal[\"%2574oken\"]" + }, + { + "name": "an escape left after the last round", + "value": { + "%2525252573ession": "x" + }, + "refusedAt": "proposal[\"%2525252573ession\"]" + }, + { + "name": "a percent sign that is not an escape", + "value": { + "100%": "x" + }, + "refusedAt": null + }, + { + "name": "fullwidth letters (NFKC)", + "value": { + "ｔｒａｎｓｃｒｉｐｔ": "x" + }, + "refusedAt": "proposal[\"ｔｒａｎｓｃｒｉｐｔ\"]" + }, + { + "name": "fullwidth upper case", + "value": { + "ＴＯＫＥＮ": "x" + }, + "refusedAt": "proposal[\"ＴＯＫＥＮ\"]" + }, + { + "name": "a compatibility ligature outside the fullwidth forms", + "value": { + "file": "x" + }, + "refusedAt": "proposal[\"file\"]" + }, + { + "name": "zero-width character", + "value": { + "pro​mpt": "x" + }, + "refusedAt": "proposal[\"pro​mpt\"]" + }, + { + "name": "Cyrillic homoglyph", + "value": { + "рrompt": "x" + }, + "refusedAt": "proposal[\"рrompt\"]" + }, + { + "name": "any non-ASCII key", + "value": { + "café": "x" + }, + "refusedAt": "proposal[\"café\"]" + }, + { + "name": "key longer than the limit (never echoed)", + "value": { + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": "x" + }, + "refusedAt": "proposal[…]" + }, + { + "name": "key at the limit", + "value": { + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa": "x" + }, + "refusedAt": null + }, + { + "name": "numeric value is exempt", + "value": { + "evidence": { + "signals": { + "sessions": 3, + "prompt_count": 2 + } + } + }, + "refusedAt": null + }, + { + "name": "string value is not exempt", + "value": { + "evidence": { + "signals": { + "sessions": "3" + } + } + }, + "refusedAt": "proposal.evidence.signals.sessions" + }, + { + "name": "boolean value is not exempt", + "value": { + "env": true + }, + "refusedAt": "proposal.env" + }, + { + "name": "null value is not exempt", + "value": { + "secret": null + }, + "refusedAt": "proposal.secret" + }, + { + "name": "embedded JSON object in a string", + "value": { + "evidence": { + "summary": "{\"transcript\": \"x\"}" + } + }, + "refusedAt": "proposal.evidence.summary#json.transcript" + }, + { + "name": "embedded JSON array in a string", + "value": { + "evidence": { + "summary": " [{\"chat\": []}]" + } + }, + "refusedAt": "proposal.evidence.summary#json[0].chat" + }, + { + "name": "embedded JSON with an escaped key", + "value": { + "evidence": { + "summary": "{\"\\u0074oken\": \"x\"}" + } + }, + "refusedAt": "proposal.evidence.summary#json.token" + }, + { + "name": "JSON inside a JSON string", + "value": { + "evidence": { + "summary": "\"{\\\"transcript\\\": \\\"x\\\"}\"" + } + }, + "refusedAt": "proposal.evidence.summary#json.transcript" + }, + { + "name": "text that is not JSON", + "value": { + "evidence": { + "summary": "{not json" + } + }, + "refusedAt": null + }, + { + "name": "a quoted word is not JSON", + "value": { + "evidence": { + "summary": "\"small steps\" help" + } + }, + "refusedAt": null + }, + { + "name": "file content fields are not parsed", + "value": { + "ops": [ + { + "patch": "{\"transcript\": \"x\"}", + "contentBase64": "[]" + } + ] + }, + "refusedAt": null + }, + { + "name": "first refusal in document order", + "value": { + "a": { + "prompt": "x" + }, + "b": { + "secret": "y" + } + }, + "refusedAt": "proposal.a.prompt" + } + ], + "localPaths": { + "refuse": [ + "\"cwd\": \"C:\\\\Users\\\\kai\\\\repo\"", + "D:\\www\\project\\notes.md", + "d:/data/file", + "share \\\\srv01\\public\\x", + "\"\\\\\\\\srv01\\\\share\\\\x\"", + "file:///home/kai/x", + "file://host/share", + "/mnt/c/Users/kai", + ">/home/kai/log", + "/dev/null", + "read /dev/stdin, write /dev/stdout and /dev/stderr", + "/USR/BIN/env", + "https://example.com/var/www/x?y=%2Fz", + "dates 12/31/2025 and 1/2", + "run /ak:plan now", + "-I/usr/include", + "-L/usr/local/lib", + "/usr/lib/node_modules/x", + "/usr/local", + "/usr/local/", + "https://example.com/usr/home/x", + "vscode://ms-python.python", + "git+https://github.com/a/b", + "ssh://git@github.com/org/repo", + "foo-bar/-draft/x", + "a -b", + "a/b//c", + "////////////////////////////////////////", + "postgres://db.internal/app", + "/proc/cpuinfo", + "/etc/ssl/certs/ca-certificates.crt", + "/usr/share/zoneinfo", + "/usr/share/doc", + "/dev/urandom", + "see /etc/hosts.", + "/usr/local/Cellar", + "/usr/local/opt", + "/etc/", + "\"\\/usr\\/bin\\/env\"", + "/usr/lib/x86_64-linux-gnu/libc.so.6", + "(https://example.com/a/b), next", + "/usr/bin/./env" + ] + }, + "secrets": [ + { + "parts": [ + "password=", + "hunter", + "22" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "db_passwd: ", + "hunter", + "22" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "\"password\": \"", + "hunter", + "22", + "\"" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "Authorization: Bearer ", + "a1a1a1a1a1a1a1a1a1a1a1a1" + ], + "rules": [ + "secret-bearer" + ] + }, + { + "parts": [ + "postgres://admin:", + "s3cretpw", + "@db.internal/x" + ], + "rules": [ + "secret-url-credentials" + ] + }, + { + "parts": [ + "x-postgres://admin:", + "s3cretpw", + "@db.internal/x" + ], + "rules": [ + "secret-url-credentials" + ] + }, + { + "parts": [ + "token ", + "eyJ", + "hbGciOiJIUzI1NiJ9", + ".eyJ", + "zdWIiOiIxIn0", + ".c2ln" + ], + "rules": [ + "secret-jwt" + ] + }, + { + "parts": [ + "auth-", + "eyJ", + "hbGciOiJIUzI1NiJ9", + ".eyJ", + "zdWIiOiIxIn0", + ".c2ln" + ], + "rules": [ + "secret-jwt" + ] + }, + { + "parts": [ + "-----BEGIN ", + "OPENSSH PRIVATE KEY-----" + ], + "rules": [ + "secret-private-key" + ] + }, + { + "parts": [ + "OPENAI_API_KEY=", + "x9x9x9x9x9x9x9x9x9x9" + ], + "rules": [ + "secret-dotenv" + ] + }, + { + "parts": [ + "export DB_PASSWORD=", + "qqqqqqqqqqqq" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "SECRET_KEY_BASE=", + "ab12ab12ab12ab12" + ], + "rules": [ + "secret-dotenv" + ] + }, + { + "parts": [ + "export github_token=", + "ab12ab12ab12ab12" + ], + "rules": [ + "secret-dotenv" + ] + }, + { + "parts": [ + "\"dbPassword\": \"", + "hunter", + "22", + "\"" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "adminPassword: ", + "hunter", + "22" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "url?smtpPwd=", + "hunter", + "22" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "const dbPassword = \"", + "hunter", + "22", + "\";" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "spring.datasource.password=", + "hunter", + "22" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "- password: ", + "hunter", + "22" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "apiKey: ", + "sk", + "-live-", + "ab12ab12ab12ab12" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "client_secret: ", + "ab12ab12ab12" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "{\"accessToken\": \"", + "ab12ab12ab12", + "\"}" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "GITHUB_TOKEN: ", + "ab12ab12ab12" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "const awsSecretAccessKey = '", + "ab12ab12ab12ab12ab12", + "'" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "let clientCredentials = ", + "ab12ab12ab12" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "Server=db;User Id=sa;Password=", + "hunter", + "22", + "!;" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "https://api.x/login?user=kai&password=", + "hunter", + "22" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "a=1,password=", + "hunter", + "22" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "mysql -u root --password=", + "hunter", + "22" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "--client-secret=", + "ab12ab12ab12" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "curl --api-key=", + "ab12ab12ab12" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "secret_key_base: ", + "ab12ab12ab12ab12ab12ab12ab12ab12" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "api_key_prod: ", + "ab12ab12ab12" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "\"private_key_id\": \"", + "ab12ab12ab12ab12ab12", + "\"" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "client_secret_v2: ", + "ab12ab12ab12" + ], + "rules": [ + "secret-assignment" + ] + }, + { + "parts": [ + "passwordValue: ", + "hunter", + "22" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "password := \"", + "hunter", + "22", + "\"" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "'password' => '", + "hunter", + "22", + "'" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "cfg[\"password\"] = \"", + "hunter", + "22", + "\"" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "password: &a !s ", + "hunter", + "22", + "secret" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "password: &a ", + "hunter", + "22", + "secret" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "password: !s ", + "hunter", + "22", + "secret" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "password: $", + "ecretP@ss" + ], + "rules": [ + "secret-password" + ] + }, + { + "parts": [ + "password: $DB_PASSWORD" + ], + "rules": [] + }, + { + "parts": [ + "token: ${{ secrets.GITHUB_TOKEN }}" + ], + "rules": [] + }, + { + "parts": [ + "apiKey: %API_KEY%" + ], + "rules": [] + }, + { + "parts": [ + "max_tokens: 4096" + ], + "rules": [] + }, + { + "parts": [ + "tokenizer: whitespace" + ], + "rules": [] + }, + { + "parts": [ + "secret_name: my-secret-value" + ], + "rules": [] + }, + { + "parts": [ + "password_file: /run/secrets/db" + ], + "rules": [] + }, + { + "parts": [ + "token: abc" + ], + "rules": [] + }, + { + "parts": [ + "password: " + ], + "rules": [] + }, + { + "parts": [ + "password=${PASSWORD}" + ], + "rules": [] + }, + { + "parts": [ + "Bearer " + ], + "rules": [] + }, + { + "parts": [ + "https://example.com/a@b" + ], + "rules": [] + }, + { + "parts": [ + "API_KEY=" + ], + "rules": [] + }, + { + "parts": [ + "the password field" + ], + "rules": [] + }, + { + "parts": [ + "AUTHOR_NAME=Jonathan" + ], + "rules": [] + }, + { + "parts": [ + "secretName: db-credentials-ref" + ], + "rules": [] + }, + { + "parts": [ + "tokenizerName: cl100k_base_x" + ], + "rules": [] + }, + { + "parts": [ + "passwordless: enabled" + ], + "rules": [] + }, + { + "parts": [ + "passwordPolicy: strict-mode" + ], + "rules": [] + }, + { + "parts": [ + "token_count: many-many" + ], + "rules": [] + }, + { + "parts": [ + "--password-file=secrets.txt" + ], + "rules": [] + }, + { + "parts": [ + "secretsManager: aws-secrets" + ], + "rules": [] + }, + { + "parts": [ + "a=a=a=a=a=a=a=a" + ], + "rules": [] + } + ], + "multilinePasswords": { + "refuse": [ + { + "parts": [ + "db:\n password:\n ", + "hunter", + "22", + "\n" + ] + }, + { + "parts": [ + "password: |\n ", + "hunter", + "22", + "\n" + ] + }, + { + "parts": [ + "password: >-\n ", + "hunter", + "22", + "\n more\n" + ] + }, + { + "parts": [ + "PASSWORD:\n\n ", + "hunter", + "22", + "\n" + ] + }, + { + "parts": [ + "users:\n - password:\n ", + "hunter", + "22", + "\n" + ] + }, + { + "parts": [ + "users:\n - name: a\n password:\n ", + "hunter", + "22", + "\n" + ] + }, + { + "parts": [ + "\"password\":\n ", + "hunter", + "22", + "\n" + ] + }, + { + "parts": [ + "'db_password': |\n ", + "hunter", + "22", + "\n" + ] + }, + { + "parts": [ + " - \"password\":\n ", + "hunter", + "22", + "\n" + ] + }, + { + "parts": [ + "password: &pw\n ", + "hunter", + "22", + "\n" + ] + }, + { + "parts": [ + "password: !secret |\n ", + "hunter", + "22", + "\n" + ] + }, + { + "parts": [ + "password:\n $", + "ecretP@ss\n" + ] + } + ], + "allow": [ + { + "parts": [ + "password: |\n \n" + ] + }, + { + "parts": [ + "password:\n ${DB_PASSWORD}\n" + ] + }, + { + "parts": [ + "password:\nnext: 1\n" + ] + }, + { + "parts": [ + "password: |\n ab\n" + ] + }, + { + "parts": [ + "- password:\n- next\n" + ] + } + ] + } +} diff --git a/packages/contracts/src/personal-kit/index.ts b/packages/contracts/src/personal-kit/index.ts index 1d2885b..00ddd7a 100644 --- a/packages/contracts/src/personal-kit/index.ts +++ b/packages/contracts/src/personal-kit/index.ts @@ -12,4 +12,6 @@ export type { TreeFacts } from "./frontmatter-facts"; export { overlayBetween, rebaseOverlay } from "./rebase"; export type { RebaseFinding, RebaseResult } from "./rebase"; export * from "./kit-document"; -export * from "./kit-lock"; \ No newline at end of file +export * from "./kit-lock"; +export * from "./proposal-policy"; + diff --git a/packages/contracts/src/personal-kit/proposal-policy.test.ts b/packages/contracts/src/personal-kit/proposal-policy.test.ts new file mode 100644 index 0000000..67586be --- /dev/null +++ b/packages/contracts/src/personal-kit/proposal-policy.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it } from "vitest"; +import vectors from "../fixtures/kit-proposal-policy-vectors-v1.json"; +import { KIT_PROPOSAL_POLICY, KIT_PROPOSAL_POLICY_SCHEMA, findForbiddenProposalKey, isForbiddenProposalKey, normalizeProposalKey, secretKeyKind } from "./proposal-policy"; + +/** + * The shared `skillx.kit-proposal-policy/v1` rules and their vectors, the + * contract AgentKit tests against. The content vectors (paths, secrets) run + * against the web detectors in `apps/web/tests/personal-kits`. + */ +const sortedUnique = (list: string[]) => [...new Set(list)].sort(); + +describe("kit proposal policy file", () => { + it("names its schema and the envelope it governs", () => { + expect(KIT_PROPOSAL_POLICY.schema).toBe(KIT_PROPOSAL_POLICY_SCHEMA); + expect(KIT_PROPOSAL_POLICY.envelope).toBe("skillx.kit-proposal/v1"); + expect(vectors.policy).toBe(KIT_PROPOSAL_POLICY_SCHEMA); + }); + + it("keeps every list sorted, unique and already normalized", () => { + const { exact, contains } = KIT_PROPOSAL_POLICY.forbiddenKeys; + for (const list of [exact, contains, KIT_PROPOSAL_POLICY.content.refusing]) expect(list).toEqual(sortedUnique(list)); + for (const name of [...exact, ...contains]) expect(normalizeProposalKey(name)).toBe(name); + }); + + it("never refuses a field of the envelope itself", () => { + const fields = ["schema", "fromRevisionId", "skill", "identity", "base", "package", "version", "releaseDigest", "overlay", "baseTreeDigest", "ops", "op", "path", "contentBase64", "mode", "expectSha256", "patch", "compositionDigest", "declaredPermissions", "evidence", "summary", "signals", "visibility"]; + expect(fields.filter(isForbiddenProposalKey)).toEqual([]); + }); +}); + +describe("forbidden key vectors", () => { + it.each(vectors.keys.map((v) => [v.name, v] as const))("%s", (_name, v) => { + const found = findForbiddenProposalKey(v.value, 32); + expect(found === null ? null : found.path).toBe(v.refusedAt); + if (found !== null) expect(found.reason).toBe("forbidden"); + }); + + it("normalizes case, separators, encodings and NFKC forms to one name", () => { + for (const key of ["systemPrompt", "SYSTEM_PROMPT", "system-prompt", "system.prompt", "System Prompt", "%73ystemPrompt", "%2573ystemPrompt", "ｓｙｓｔｅｍＰｒｏｍｐｔ"]) { + expect(normalizeProposalKey(key)).toBe("systemprompt"); + } + expect(normalizeProposalKey("pro​mpt")).toBeNull(); + expect(normalizeProposalKey("%E2%80%8Bprompt")).toBeNull(); + expect(normalizeProposalKey("x".repeat(65))).toBeNull(); + // The length counts code points, so 64 astral characters are within the limit (and refused as non-ASCII). + expect([..."😀".repeat(64)].length).toBe(64); + expect(normalizeProposalKey("😀".repeat(64))).toBeNull(); + }); + + it("folds only fullwidth forms through NFKC and pins the Unicode version", () => { + expect(KIT_PROPOSAL_POLICY.forbiddenKeys.unicodeVersion).toBe("15.0"); + expect(normalizeProposalKey("ＴＯＫＥＮ")).toBe("token"); + // A compatibility ligature outside the fullwidth block is refused, not folded. + expect(normalizeProposalKey("file")).toBeNull(); + expect(normalizeProposalKey("%2525252573ession")).toBeNull(); + expect(normalizeProposalKey("100%")).toBe("100"); + }); + + it("writes unambiguous key paths and never echoes a long key", () => { + expect(findForbiddenProposalKey({ "a.b": { transcript: "x" } }, 32)?.path).toBe('proposal["a.b"].transcript'); + expect(findForbiddenProposalKey({ "a[0]": { transcript: "x" } }, 32)?.path).toBe('proposal["a[0]"].transcript'); + expect(findForbiddenProposalKey({ ["t".repeat(65)]: "x" }, 32)?.path).toBe("proposal[…]"); + }); + + it("bounds the nesting, also through embedded JSON", () => { + let deep: unknown = {}; + for (let i = 0; i < 40; i++) deep = { a: deep }; + expect(findForbiddenProposalKey(deep, 32)).toMatchObject({ reason: "too_deep" }); + let text = "{}"; + // Each level is an object holding the previous level as a JSON string (escaping doubles per level). + for (let i = 0; i < 12; i++) text = JSON.stringify({ a: text }); + expect(findForbiddenProposalKey({ summary: text }, 32)).toBeNull(); + expect(findForbiddenProposalKey({ summary: text }, 8)).toMatchObject({ reason: "too_deep" }); + }); +}); + +describe("secret key names of the content rules", () => { + it("names password and secret keys in any case and separator style", () => { + for (const key of ["password", "dbPassword", "DB_PASSWORD", "db-passwd", "userPwd", "sshPassphrase"]) expect(secretKeyKind(key)).toBe("password"); + for (const key of ["apiKey", "API_KEY", "clientSecret", "client_secret", "GITHUB_TOKEN", "accessToken", "awsCredentials", "privateKey"]) expect(secretKeyKind(key)).toBe("secret"); + }); + + it("matches a stem at the end of the key, anywhere only on a dotenv line", () => { + expect(secretKeyKind("tokenizer")).toBeNull(); + expect(secretKeyKind("passwordHint")).toBeNull(); + expect(secretKeyKind("TOKEN_URL")).toBeNull(); + expect(secretKeyKind("TOKEN_URL", true)).toBe("secret"); + expect(secretKeyKind("PASSWORD_FILE", true)).toBe("password"); + }); + + it("allows at most two qualifiers after a stem", () => { + for (const key of ["secret_key_base", "api_key_prod", "private_key_id", "client_secret_v2", "passwordValue", "API_KEY_PROD_V2"]) expect(secretKeyKind(key)).not.toBeNull(); + for (const key of ["secretName", "secretsManager", "passwordless", "passwordPolicy", "token_count", "api_key_prod_v2_old"]) expect(secretKeyKind(key)).toBeNull(); + }); + + it("treats a key that cannot be normalized as a secret key", () => { + expect(secretKeyKind("pаssword")).toBe("secret"); + expect(secretKeyKind("k".repeat(65))).toBe("secret"); + }); +}); diff --git a/packages/contracts/src/personal-kit/proposal-policy.ts b/packages/contracts/src/personal-kit/proposal-policy.ts new file mode 100644 index 0000000..91495ce --- /dev/null +++ b/packages/contracts/src/personal-kit/proposal-policy.ts @@ -0,0 +1,126 @@ +/** + * `skillx.kit-proposal-policy/v1`: the forbidden-key rules of a + * `skillx.kit-proposal/v1` envelope and the secret-key names of the content + * rules, shared with AgentKit, which refuses the same keys locally before it + * uploads a proposal. The single source is `fixtures/kit-proposal-policy-v1.json`; + * both sides test against `fixtures/kit-proposal-policy-vectors-v1.json`. This + * module only applies the JSON rules, so a list change is a JSON change. + */ +import policy from "../fixtures/kit-proposal-policy-v1.json"; + +export const KIT_PROPOSAL_POLICY_SCHEMA = "skillx.kit-proposal-policy/v1"; +export const KIT_PROPOSAL_POLICY = policy; + +const EXACT = new Set(policy.forbiddenKeys.exact); +const CONTAINS = policy.forbiddenKeys.contains; +const CONTENT_FIELDS = new Set(policy.forbiddenKeys.contentFields); +const MAX_KEY_LENGTH = policy.forbiddenKeys.maxKeyLength; +const ROUNDS = policy.forbiddenKeys.percentDecodeRounds; +const ESCAPE = /%[0-9a-f]{2}/i; + +/** Decodes `%XX` escapes (each to the code unit of its byte) until nothing changes; null when escapes remain after the last round. */ +function percentDecode(key: string): string | null { + let out = key; + for (let i = 0; i < ROUNDS && ESCAPE.test(out); i++) out = out.replace(/%([0-9a-f]{2})/gi, (_, h: string) => String.fromCharCode(parseInt(h, 16))); + return ESCAPE.test(out) ? null : out; +} + +/** + * The comparable form of a key (`[a-z0-9]*`), or null when the key cannot be + * checked: longer than the limit, still percent-encoded after the last round, + * or holding a character other than printable ASCII and its fullwidth forms + * (zero-width characters, homoglyphs, any other script). Only fullwidth + * forms reach NFKC, so the result does not depend on the Unicode version. + */ +export function normalizeProposalKey(key: string): string | null { + if ([...key].length > MAX_KEY_LENGTH) return null; + const decoded = percentDecode(key); + if (decoded === null || !/^[\x20-\x7e！-～]*$/.test(decoded)) return null; + const folded = decoded.normalize("NFKC"); + if (!/^[\x20-\x7e]*$/.test(folded)) return null; + return folded.replace(/[A-Z]/g, (c) => c.toLowerCase()).replace(/[^a-z0-9]/g, ""); +} + +/** True when a key is refused whatever its (non-numeric) value. */ +export function isForbiddenProposalKey(key: string): boolean { + const normal = normalizeProposalKey(key); + return normal === null || EXACT.has(normal) || CONTAINS.some((stem) => normal.includes(stem)); +} + +const SECRET_KEYS = policy.content.secretKeys; +/** A stem followed by at most two qualifiers (`base`, `prod`, `id`, `v2`, ...) at the end of a normalized key. */ +const stemAtEnd = (stems: string[]) => new RegExp(`(?:${stems.join("|")})(?:${SECRET_KEYS.qualifiers.join("|")}|v?[0-9]+){0,2}$`); +const PASSWORD_KEY = stemAtEnd(SECRET_KEYS.password); +const SECRET_KEY = stemAtEnd(SECRET_KEYS.secret); + +/** + * What a key in file content (`key: value`, `key = value`, `"key": value`) + * names: `"password"` or `"secret"` when its normalized form holds one of the + * policy stems followed by nothing or by at most two qualifiers + * (`secret_key_base`, `api_key_prod`, `private_key_id`), `null` otherwise + * (`secretName`, `tokenizer`). `dotenv` (a `KEY=value` line) matches a stem + * anywhere in the key. A key that cannot be normalized names a secret. + */ +export function secretKeyKind(key: string, dotenv = false): "password" | "secret" | null { + const normal = normalizeProposalKey(key); + if (normal === null) return "secret"; + const names = (stems: string[], atEnd: RegExp) => (dotenv ? stems.some((s) => normal.includes(s)) : atEnd.test(normal)); + if (names(SECRET_KEYS.password, PASSWORD_KEY)) return "password"; + return names(SECRET_KEYS.secret, SECRET_KEY) ? "secret" : null; +} + +export type ProposalKeyFinding = { reason: "forbidden"; path: string } | { reason: "too_deep"; path: string }; + +/** Parses a string that holds a JSON object or array (also through JSON string layers), or returns undefined. */ +function embeddedJson(text: string): unknown { + let t = text.trimStart(); + for (let layer = 0; layer < ROUNDS + 1; layer++) { + if (!t.startsWith("{") && !t.startsWith("[") && !t.startsWith('"')) return undefined; + let parsed: unknown; + try { + parsed = JSON.parse(t) as unknown; + } catch { + return undefined; + } + if (typeof parsed !== "string") return parsed; + t = parsed.trimStart(); + } + return undefined; +} + +/** + * One key of a path: `.key` for `[A-Za-z0-9_$-]+`, otherwise `["key"]` (a + * JSON string), and `[…]` for a key longer than the limit, which is never echoed. + */ +function keySegment(key: string): string { + if (/^[A-Za-z0-9_$-]+$/.test(key) && key.length <= MAX_KEY_LENGTH) return `.${key}`; + return [...key].length > MAX_KEY_LENGTH ? "[…]" : `[${JSON.stringify(key)}]`; +} + +/** + * The first refused key of `value` in document order (its path, never its + * value), or the first place it nests deeper than `maxDepth`; null when the + * document passes. A key with a number value is exempt. A string outside the + * file-content fields that holds a JSON object or array is checked as a + * nested document at `#json`. Paths use `.key`, `["key"]` and `[index]`. + */ +export function findForbiddenProposalKey(value: unknown, maxDepth: number, at = "proposal", depth = 0): ProposalKeyFinding | null { + if (typeof value === "object" && value !== null && depth >= maxDepth) return { reason: "too_deep", path: at }; + if (Array.isArray(value)) { + for (let i = 0; i < value.length; i++) { + const found = findForbiddenProposalKey(value[i], maxDepth, `${at}[${i}]`, depth + 1); + if (found !== null) return found; + } + return null; + } + if (typeof value !== "object" || value === null) return null; + for (const [key, child] of Object.entries(value)) { + const path = at + keySegment(key); + if (typeof child === "number") continue; + if (isForbiddenProposalKey(key)) return { reason: "forbidden", path }; + const nested = typeof child === "string" && !CONTENT_FIELDS.has(key) ? embeddedJson(child) : undefined; + const found = nested === undefined ? findForbiddenProposalKey(child, maxDepth, path, depth + 1) : findForbiddenProposalKey(nested, maxDepth, `${path}#json`, depth + 1); + if (found !== null) return found; + } + return null; +}