From aa839b4e2eba34daccef90b386e08f2c4b425fca Mon Sep 17 00:00:00 2001 From: Korivi Date: Mon, 7 Sep 2026 18:57:53 +0900 Subject: [PATCH 1/6] Guard fields named after prototype members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard is keyed by names taken straight from the request body, and it probed two plain objects with them. `allow["constructor"]` and `map["toString"]` are truthy on any plain object, so a field named after an Object.prototype member skipped the allow check, then read as declared, then passed every rule after it — no unknown_field, no type check, no enum check, no read-only check. The key went on to the backend. Confirmed against a running app: `bogus` was rejected, while `constructor`, `toString`, `hasOwnProperty`, `valueOf` and `__proto__` were all accepted with no violation at all. This is the one guarantee the rules layer exists to make ("no silent 200s"), voided by nothing more than the field's name, in the path shared by every adapter — and, since operation arguments now run through the same rules, by every declared operation too. Make the lookups own-property tests and give the field map a null prototype. Object.prototype pollution itself was not reachable: JSON.parse makes __proto__ an ordinary own property, and neither spread nor Object.assign writes through it. --- adapters/adapter-core/src/server.ts | 5 ++++- adapters/rules/src/index.ts | 21 ++++++++++++++++++--- conformance/suites/class-a-core.yaml | 18 ++++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/adapters/adapter-core/src/server.ts b/adapters/adapter-core/src/server.ts index 0fdf2ac..4b38417 100644 --- a/adapters/adapter-core/src/server.ts +++ b/adapters/adapter-core/src/server.ts @@ -504,7 +504,10 @@ export function createA2App(binding: Binding, config: A2AppConfig): A2App { // Guard the RAW body before any backend coercion. Required-field presence is // enforced on CREATE only (POST) — a PATCH is a legitimate partial write. - const allow: Record = {}; + // Null prototype: `allow` is probed with keys from the request body, and on a + // plain object `allow["constructor"]` is truthy, which makes validate() skip + // that field entirely rather than guard it. + const allow: Record = Object.create(null) as Record; for (const k of def.writeAllow ?? []) allow[k] = true; const violations = validate(def.fields, body, { allow, requireRequired: req.method === "POST" }); if (violations.length) { diff --git a/adapters/rules/src/index.ts b/adapters/rules/src/index.ts index 8e53a1c..463b82c 100644 --- a/adapters/rules/src/index.ts +++ b/adapters/rules/src/index.ts @@ -196,8 +196,16 @@ export function isDayKeyField(field: Pick { - const m: Record = {}; + const m: Record = Object.create(null) as Record; for (const f of fields) m[f.name] = f; return m; } @@ -244,11 +252,18 @@ export function validate( const map = fieldMap(fields); const writable = writableNames(fields); const out: Violation[] = []; + // Every lookup below is keyed by a name taken from the request body, so it + // must be an OWN-property test. `allow["constructor"]` and `map["toString"]` + // are truthy on any plain object, which let a field named after an + // Object.prototype member skip the allow check, read as declared, and pass + // every remaining rule — the guard silently voided by the field's name. + const has = (o: Record | Record, k: string): boolean => + Object.prototype.hasOwnProperty.call(o, k); for (const key of Object.keys(body)) { - if (allow[key]) continue; + if (has(allow, key) && allow[key]) continue; - const field = map[key]; + const field = has(map, key) ? map[key] : undefined; // Unknown fields are silently dropped by most backends and absent from the // response, so the caller cannot tell they were ignored. diff --git a/conformance/suites/class-a-core.yaml b/conformance/suites/class-a-core.yaml index 02904a7..d6d9639 100644 --- a/conformance/suites/class-a-core.yaml +++ b/conformance/suites/class-a-core.yaml @@ -169,6 +169,24 @@ checks: match: { a2app: true, ok: false } len: { violations: 3 } + # A field named after an Object.prototype member must not read as declared. + # On a plain-object field map `map["constructor"]` is truthy, which passed the + # unknown-field check and every rule after it, so the key reached the backend. + - name: a field named after a prototype member is still unknown + request: + method: POST + path: /api/collections/cards/records + token: full + body: { title: "x", constructor: "evil" } + expect: + status: 400 + match: + a2app: true + ok: false + violations.0.code: unknown_field + violations.0.field: constructor + len: { violations: 1 } + - name: guard writes to a read-only field are refused request: method: POST From 47a36ef1cadc3a1139b9f53052c8219fdd03809c Mon Sep 17 00:00:00 2001 From: Korivi Date: Mon, 7 Sep 2026 19:01:12 +0900 Subject: [PATCH 2/6] Bind adapters to loopback, as they already claimed to `server.listen(PORT, cb)` binds every interface. Three shipped servers did that while printing a startup line that said 127.0.0.1, including the blueprint template that every scaffolded UI app runs. That matters more than a wrong log line, because a same-origin request is trusted as the owner without any credential, and the origins those apps allow are their own guessable localhost URLs. The two together made a scaffolded Agent App readable and writable by anyone who could reach the port, with no token: curl -X POST http://:8090/api/collections/tasks/records \ -H 'Origin: http://127.0.0.1:8090' -d '{"title":"..."}' -> 200 Verified against a real scaffolded app before the change (bound 0.0.0.0, wrote a record over the LAN address) and after (binds 127.0.0.1, remote connection refused, loopback unaffected). The reverse proxy mattered most: it carries an owner-scoped grant with scopes ["*"]. The README already promises v0.1 is loopback-trust only. This makes the code keep that promise; A2APP_HOST is there so exposing an app stays a deliberate act. --- adapters/adapter-starter/src/index.ts | 4 +++- adapters/reverse-proxy/src/index.ts | 5 ++++- toolkits/blueprint-react-node/template/server.mjs | 10 ++++++++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/adapters/adapter-starter/src/index.ts b/adapters/adapter-starter/src/index.ts index e064e84..c0ba928 100644 --- a/adapters/adapter-starter/src/index.ts +++ b/adapters/adapter-starter/src/index.ts @@ -78,7 +78,9 @@ export function startNotesApp(port = 8091, token = "a2app_starter_token"): Retur allowedOrigins: [`http://localhost:${port}`, `http://127.0.0.1:${port}`], }); const server = createA2AppServer(app); - server.listen(port, () => process.stdout.write(`notes adapter on http://127.0.0.1:${port}\n`)); + // Loopback explicitly: `listen(port)` alone binds every interface, which this + // line already claimed it did not. + server.listen(port, "127.0.0.1", () => process.stdout.write(`notes adapter on http://127.0.0.1:${port}\n`)); return server; } diff --git a/adapters/reverse-proxy/src/index.ts b/adapters/reverse-proxy/src/index.ts index abce02e..3ec63ab 100644 --- a/adapters/reverse-proxy/src/index.ts +++ b/adapters/reverse-proxy/src/index.ts @@ -182,6 +182,9 @@ export function createReverseProxy(cfg: ReverseProxyConfig, credentials: Grant[] export function startReverseProxy(cfg: ReverseProxyConfig, port: number, token: string): ReturnType { const grant: Grant = { token, credentialId: "cred_proxy", agentName: "local", principal: "owner", scopes: ["*"] }; const server = createA2AppServer(createReverseProxy(cfg, [grant])); - server.listen(port, () => process.stdout.write(`reverse-proxy adapter for ${cfg.appId} on http://127.0.0.1:${port}\n`)); + // Loopback explicitly: `listen(port)` alone binds every interface, which this + // line already claimed it did not. A sidecar carrying an owner-scoped grant + // must not be the thing that puts the app on the network. + server.listen(port, "127.0.0.1", () => process.stdout.write(`reverse-proxy adapter for ${cfg.appId} on http://127.0.0.1:${port}\n`)); return server; } diff --git a/toolkits/blueprint-react-node/template/server.mjs b/toolkits/blueprint-react-node/template/server.mjs index d4e0759..026e535 100644 --- a/toolkits/blueprint-react-node/template/server.mjs +++ b/toolkits/blueprint-react-node/template/server.mjs @@ -241,6 +241,12 @@ function serveStatic(req, res) { } const server = createA2AppServer(app, serveStatic); -server.listen(PORT, () => { - process.stdout.write(`Agent App "${manifest.name ?? manifest.id}" on http://localhost:${PORT} (A2App id ${manifest.id})\n`); +// Bind loopback explicitly. `listen(PORT)` alone binds every interface, so the +// app was reachable from the network while its own log line said localhost -- +// and a same-origin request is trusted as the owner without a credential, which +// made a scaffolded Agent App remotely writable by anyone who could reach the +// port. Exposing it must be a deliberate act, hence the env var. +const HOST = process.env.A2APP_HOST ?? "127.0.0.1"; +server.listen(PORT, HOST, () => { + process.stdout.write(`Agent App "${manifest.name ?? manifest.id}" on http://${HOST}:${PORT} (A2App id ${manifest.id})\n`); }); From 2a371abb39066566498c4c9d569a8e62fdb9a9ad Mon Sep 17 00:00:00 2001 From: Korivi Date: Mon, 7 Sep 2026 19:48:54 +0900 Subject: [PATCH 3/6] Contain vendored writes physically, not lexically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit assertInside compares strings, which is not the question a write needs answered. A path with no `..` and no drive letter is "inside" by that test even when a component of it is a link pointing somewhere else, and an app directory is untrusted input: `import` exists to take one from a stranger, and tar, zip and git all carry links — a Windows junction needs no privilege at all. So a shipped app carrying `hooks -> ~/.ssh`, plus a toolkit declaring `hooks/authorized_keys` among its system paths, had that file vendored straight through the link. Reproduced here: assertInside answered "contained" and the write replaced a file outside the project. vendorPaths now resolves containment physically — every component below the project is checked, while the project directory itself may still be a link, since the user chose where it lives. copyTree also replaces a link it finds at a destination rather than writing through it, so a link left in the tree cannot redirect a later sync. The lexical check keeps its own name and behaviour; it is still the right test for a path that is never opened. --- framework/cli/package.json | 2 +- framework/cli/src/lib/fsx.ts | 44 +++++++++++++- framework/cli/src/lib/toolkit.ts | 4 +- framework/cli/test/containment.test.mjs | 80 +++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 framework/cli/test/containment.test.mjs diff --git a/framework/cli/package.json b/framework/cli/package.json index 246a1f4..5c75218 100644 --- a/framework/cli/package.json +++ b/framework/cli/package.json @@ -35,7 +35,7 @@ "scripts": { "build": "tsc -p tsconfig.json", "typecheck": "tsc -p tsconfig.json --noEmit", - "test": "tsc -p tsconfig.json && node test/copy-tree.test.mjs", + "test": "tsc -p tsconfig.json && node test/copy-tree.test.mjs && node test/containment.test.mjs", "bundle-blueprints": "node scripts/bundle-blueprints.mjs", "bundle-skills": "node scripts/bundle-skills.mjs", "bundle": "node scripts/bundle-blueprints.mjs && node scripts/bundle-skills.mjs", diff --git a/framework/cli/src/lib/fsx.ts b/framework/cli/src/lib/fsx.ts index b4f8b02..ac9982d 100644 --- a/framework/cli/src/lib/fsx.ts +++ b/framework/cli/src/lib/fsx.ts @@ -22,7 +22,7 @@ import { statSync, symlinkSync, } from "node:fs"; -import { dirname, isAbsolute, join, relative, resolve } from "node:path"; +import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; /** A monotonic-per-process suffix so staged temp names never collide, without * relying on wall-clock time. */ @@ -31,6 +31,41 @@ function tmpSuffix(): string { return `${process.pid}-${tmpCounter++}`; } +/** + * Physical containment: lexically inside `base` AND not reached through a + * symlink. + * + * {@link assertInside} compares strings, so a path with no `..` and no drive + * letter passes even when a component of it is a link pointing elsewhere. An + * app directory is untrusted input — `import` exists to take one from a + * stranger, and tar, zip and git all carry links (a Windows junction needs no + * privilege at all) — so a template file named `hooks/authorized_keys`, with + * `hooks` a link to ~/.ssh, was written straight through the link and + * overwrote a file the project has no business touching. + * + * Every component below `base` is checked. `base` itself may legitimately be a + * link: the user chose where their project lives. + */ +export function assertRealInside(base: string, candidate: string, label = "path"): string { + const target = assertInside(base, candidate, label); + const root = resolve(base); + let cur = root; + for (const segment of relative(root, target).split(sep)) { + if (segment === "") continue; + cur = join(cur, segment); + let st; + try { + st = lstatSync(cur); + } catch { + break; // nothing here yet, so nothing to traverse + } + if (st.isSymbolicLink()) { + throw new Error(`${label} "${candidate}" leaves ${root} through a link at ${cur}`); + } + } + return target; +} + /** * Resolve `candidate` against `base` and throw if it escapes `base` (a `..` * traversal or an absolute path pointing elsewhere). Returns the absolute, @@ -106,6 +141,13 @@ export function copyTree(src: string, dest: string): void { } if (!st.isDirectory()) { mkdirSync(dirname(dest), { recursive: true }); + // Replace a link rather than write through it: copyFileSync follows the + // destination, so a link left in the tree redirects the write outside it. + try { + if (lstatSync(dest).isSymbolicLink()) rmSync(dest, { force: true }); + } catch { + /* nothing there yet */ + } copyFileSync(src, dest); return; } diff --git a/framework/cli/src/lib/toolkit.ts b/framework/cli/src/lib/toolkit.ts index 51bbcb1..6a58bb0 100644 --- a/framework/cli/src/lib/toolkit.ts +++ b/framework/cli/src/lib/toolkit.ts @@ -24,7 +24,7 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { dirname, isAbsolute, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; -import { assertInside, copyTree } from "./fsx.js"; +import { assertInside, assertRealInside, copyTree } from "./fsx.js"; export interface GateStep { name: string; @@ -121,7 +121,7 @@ export function vendorPaths(tk: ResolvedToolkit, projectDir: string, paths: stri // the project. assertInside throws on a `../` or absolute escape. const src = assertInside(templateRoot, rel, "template path"); if (!existsSync(src)) continue; - const dest = assertInside(projectDir, rel, "vendored path"); + const dest = assertRealInside(projectDir, rel, "vendored path"); mkdirSync(dirname(dest), { recursive: true }); copyTree(src, dest); written.push(rel); diff --git a/framework/cli/test/containment.test.mjs b/framework/cli/test/containment.test.mjs new file mode 100644 index 0000000..d2f0164 --- /dev/null +++ b/framework/cli/test/containment.test.mjs @@ -0,0 +1,80 @@ +/** + * Containment: a write aimed into a project must land in that project. + * + * `assertInside` compares strings, which is not the same question. An app + * directory is untrusted — `import` exists to take one from a stranger, and + * tar, zip and git all carry links — so a path with no `..` in it can still be + * a link pointing at the user's home directory. This file pins the physical + * check, and the copy that must not follow a link it finds at the destination. + * + * Symlink creation needs a privilege Windows does not grant by default; when it + * is unavailable the cases that need one are skipped rather than failed, so + * this still runs everywhere (CI is Linux, where they always work). + */ +import { lstatSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { assertInside, assertRealInside, copyTree } from "../dist/lib/fsx.js"; + +const failures = []; +let skipped = 0; +const check = (label, got, want) => { + if (got !== want) failures.push(`${label}\n expected: ${want}\n actual: ${got}`); +}; +const throws = (label, fn) => { + try { fn(); failures.push(`${label}\n expected: a thrown error\n actual: completed`); } + catch { /* expected */ } +}; + +const base = mkdtempSync(join(tmpdir(), "containment-")); +const project = join(base, "project"); +const outside = join(base, "outside"); +mkdirSync(project, { recursive: true }); +mkdirSync(outside, { recursive: true }); +writeFileSync(join(outside, "authorized_keys"), "ORIGINAL"); + +// Lexical containment still rejects the obvious escapes. +throws("a parent-relative path is refused", () => assertInside(project, "../outside/x", "p")); +throws("an absolute path elsewhere is refused", () => assertInside(project, join(outside, "x"), "p")); +check("an ordinary relative path is accepted", + typeof assertInside(project, "reference/requirements.md", "p"), "string"); +check("the physical check agrees on an ordinary path", + typeof assertRealInside(project, "reference/requirements.md", "p"), "string"); + +let links = true; +try { + symlinkSync(outside, join(project, "hooks"), "junction"); +} catch { + links = false; + skipped += 2; +} + +if (links) { + // The whole point: lexically clean, physically outside. + throws("a path reached through a linked directory is refused", () => + assertRealInside(project, "hooks/authorized_keys", "vendored path")); + + // And a copy must replace a link at the destination, not write through it. + const src = join(base, "payload.txt"); + writeFileSync(src, "VENDORED"); + const linkedDest = join(project, "linked-file"); + try { + symlinkSync(join(outside, "authorized_keys"), linkedDest, "file"); + copyTree(src, linkedDest); + check("the file outside is untouched", readFileSync(join(outside, "authorized_keys"), "utf8"), "ORIGINAL"); + check("the destination is no longer a link", lstatSync(linkedDest).isSymbolicLink(), false); + check("the destination holds the copied bytes", readFileSync(linkedDest, "utf8"), "VENDORED"); + } catch (e) { + if (e && e.code === "EPERM") skipped += 3; + else throw e; + } +} + +rmSync(base, { recursive: true, force: true }); + +if (failures.length) { + console.log(`containment: ${failures.length} check(s) FAILED\n`); + for (const f of failures) console.log(` [FAIL] ${f}\n`); + process.exit(1); +} +console.log(`containment: all checks passed${skipped ? ` (${skipped} skipped: symlinks unavailable)` : ""}`); From 1f260f1d0490d537451e9e366563eaea3dbb488a Mon Sep 17 00:00:00 2001 From: Korivi Date: Mon, 7 Sep 2026 19:52:03 +0900 Subject: [PATCH 4/6] Answer only on this app's own host Binding to loopback does not keep a browser out. A page on evil.com whose DNS re-resolves to 127.0.0.1 is, to the browser, same-origin with this server: it sends no Origin on a GET, so the origin rule has nothing to judge, and on a single-user app an anonymous caller already reads everything. The page walks describe, the records API, events and tasks, and posts them anywhere. The Host header is what still distinguishes the two: a rebound request carries `evil.com`, because that is the name the page was fetched from. Check it once at the router entry, ahead of the identity document, which is otherwise a free fingerprint for exactly that page. Loopback by address, by name and by IPv6 literal are served; the hosts of any declared allowedOrigins are served, so an app that configures its own UI origin is unaffected; allowedHosts covers a deliberate exception. A lookalike like `localhost.evil.com` is not served. Tested over a raw socket rather than in the conformance suite, because fetch forbids setting Host and a rebinding attacker is a browser that sets it implicitly. --- adapters/adapter-core/package.json | 70 ++++++++++++++---------- adapters/adapter-core/src/server.ts | 36 ++++++++++++ adapters/adapter-core/src/types.ts | 3 + adapters/adapter-core/test/host.test.mjs | 63 +++++++++++++++++++++ 4 files changed, 142 insertions(+), 30 deletions(-) create mode 100644 adapters/adapter-core/test/host.test.mjs diff --git a/adapters/adapter-core/package.json b/adapters/adapter-core/package.json index fd50c34..5f21783 100644 --- a/adapters/adapter-core/package.json +++ b/adapters/adapter-core/package.json @@ -1,32 +1,42 @@ { - "name": "@a2app/adapter-core", - "version": "0.1.0", - "description": "Stack-agnostic A2App served surface: identity, describe, guard+read-back, operations, tasks/events, context, whoami, IAM — driven by a pluggable backend binding", - "license": "MIT", - "repository": { "type": "git", "url": "git+https://github.com/CraftOS-dev/Agent-App.git", "directory": "adapters/adapter-core" }, - "homepage": "https://github.com/CraftOS-dev/Agent-App#readme", - "bugs": "https://github.com/CraftOS-dev/Agent-App/issues", - "publishConfig": { "access": "public" }, - "type": "module", - "main": "./dist/index.js", - "types": "./dist/index.d.ts", - "exports": { - ".": { - "types": "./dist/index.d.ts", - "default": "./dist/index.js" - } - }, - "files": ["dist"], - "engines": { "node": ">=20" }, - "dependencies": { - "@a2app/rules": "workspace:*" - }, - "devDependencies": { - "@types/node": "^20.19.0" - }, - "scripts": { - "build": "tsc -p tsconfig.json", - "typecheck": "tsc -p tsconfig.json --noEmit", - "test": "echo \"no tests\" && exit 0" - } + "name": "@a2app/adapter-core", + "version": "0.1.0", + "description": "Stack-agnostic A2App served surface: identity, describe, guard+read-back, operations, tasks/events, context, whoami, IAM — driven by a pluggable backend binding", + "license": "MIT", + "repository": { + "type": "git", + "url": "git+https://github.com/CraftOS-dev/Agent-App.git", + "directory": "adapters/adapter-core" + }, + "homepage": "https://github.com/CraftOS-dev/Agent-App#readme", + "bugs": "https://github.com/CraftOS-dev/Agent-App/issues", + "publishConfig": { + "access": "public" + }, + "type": "module", + "main": "./dist/index.js", + "types": "./dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "default": "./dist/index.js" + } + }, + "files": [ + "dist" + ], + "engines": { + "node": ">=20" + }, + "dependencies": { + "@a2app/rules": "workspace:*" + }, + "devDependencies": { + "@types/node": "^20.19.0" + }, + "scripts": { + "build": "tsc -p tsconfig.json", + "typecheck": "tsc -p tsconfig.json --noEmit", + "test": "tsc -p tsconfig.json && node test/host.test.mjs" + } } diff --git a/adapters/adapter-core/src/server.ts b/adapters/adapter-core/src/server.ts index 4b38417..7cfb2e3 100644 --- a/adapters/adapter-core/src/server.ts +++ b/adapters/adapter-core/src/server.ts @@ -92,6 +92,16 @@ export function createA2App(binding: Binding, config: A2AppConfig): A2App { const opByName = new Map(operations.map((o) => [o.name, o])); const eventTypes = knownEventTypes(config.events); const allowedOrigins = new Set(config.allowedOrigins ?? []); + // Host names this app will answer to. Derived from the origins it already + // declares, so an app that configures its own UI origin keeps working. + const allowedHosts = new Set((config.allowedHosts ?? []).map((h) => h.toLowerCase())); + for (const origin of config.allowedOrigins ?? []) { + try { + allowedHosts.add(new URL(origin).host.toLowerCase()); + } catch { + /* not a URL - ignore */ + } + } const audit = config.audit ?? ((e: AuditEntry) => store.appendAudit(e)); const limiter = new RateLimiter({ ...DEFAULT_RATE_LIMITS, ...(config.rateLimits ?? {}) }, () => now().getTime()); @@ -324,6 +334,25 @@ export function createA2App(binding: Binding, config: A2AppConfig): A2App { return store.grantByToken(token); } + /** + * Is this request addressed to the loopback interface by name? + * + * Binding to 127.0.0.1 does not stop a browser reaching the app: an attacker + * page on evil.com whose DNS re-resolves to 127.0.0.1 is then *same-origin* + * with this server as far as the browser is concerned, so it sends no Origin + * on a GET and can read every response. The origin rule cannot catch that — + * there is no origin to judge. The Host header can: a rebound request still + * carries `evil.com`, because that is the name the page was fetched from. + */ + function isAllowedHost(raw: string): boolean { + const host = raw.toLowerCase(); + if (allowedHosts.has(host)) return true; + // Strip the port; an IPv6 literal is bracketed, so scan past the bracket. + const name = host.startsWith("[") ? host.slice(0, host.indexOf("]") + 1) : (host.split(":")[0] ?? ""); + if (name === "localhost" || name === "[::1]" || name === "::1") return true; + return /^127\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(name); + } + function isForeignOrigin(req: A2AppRequest): boolean { const origin = req.headers["origin"]; if (origin === undefined) return false; // a program, not a browser @@ -793,6 +822,13 @@ export function createA2App(binding: Binding, config: A2AppConfig): A2App { async function handle(req: A2AppRequest): Promise { const path = req.path.replace(/\/+$/, "") || "/"; + // Before any route, including the unauthenticated identity document, which + // is otherwise a free fingerprint of the app for a rebound page. + const hostHeader = req.headers["host"]; + if (hostHeader !== undefined && !isAllowedHost(hostHeader)) { + return err(403, "forbidden_host", `Refused: this app answers only on its own host, not "${hostHeader}".`); + } + // Identity (unauthenticated). if (path === "/.well-known/a2app.json" || path === "/api/_a2app") { return ok(identityDoc()); diff --git a/adapters/adapter-core/src/types.ts b/adapters/adapter-core/src/types.ts index 5955a09..7020882 100644 --- a/adapters/adapter-core/src/types.ts +++ b/adapters/adapter-core/src/types.ts @@ -238,6 +238,9 @@ export interface A2AppConfig { conventions?: Record; /** the app's own origins; a write from any other Origin is refused 403. */ allowedOrigins?: string[]; + /** Host names to answer on, beyond loopback and the hosts of allowedOrigins. + * Set this only when the app is deliberately reachable under another name. */ + allowedHosts?: string[]; /** declared event types. */ events?: EventTypeDecl[]; /** injectable clock, for deterministic tests. */ diff --git a/adapters/adapter-core/test/host.test.mjs b/adapters/adapter-core/test/host.test.mjs new file mode 100644 index 0000000..1a3a1af --- /dev/null +++ b/adapters/adapter-core/test/host.test.mjs @@ -0,0 +1,63 @@ +/** + * Host validation, exercised over a raw socket. + * + * This cannot live in the conformance suite: that harness drives the CLI and + * fetch, and fetch forbids setting `Host` — the header this rule is about. A + * rebinding attacker is a browser and sets it implicitly, so the only faithful + * client here is one that writes the request itself. + */ +import { createServer, request } from "node:http"; +import { createA2App, MemoryBinding } from "../dist/index.js"; +import { a2appMiddleware } from "../dist/http.js"; + +const failures = []; +const check = (label, got, want) => { + if (got !== want) failures.push(`${label}\n expected: ${want}\n actual: ${got}`); +}; + +const app = createA2App( + new MemoryBinding({ + appId: "host_test", + appName: "Host Test", + entities: { notes: { module: "desk", fields: [{ name: "title", type: "string", required: true }], seed: [] } }, + }), + { + modules: [{ name: "desk", summary: "notes" }], + allowedOrigins: ["http://localhost:9931", "http://127.0.0.1:9931"], + }, +); + +const mw = a2appMiddleware(app); +const server = createServer((req, res) => + mw(req, res, () => { res.writeHead(404); res.end("{}"); }), +); +await new Promise((r) => server.listen(0, "127.0.0.1", r)); +const port = server.address().port; + +const get = (hostHeader) => + new Promise((resolve) => { + const req = request( + { host: "127.0.0.1", port, path: "/api/_a2app", method: "GET", headers: { host: hostHeader } }, + (res) => { res.resume(); resolve(res.statusCode); }, + ); + req.on("error", () => resolve(-1)); + req.end(); + }); + +check("a rebound foreign host is refused", await get("evil.example.com"), 403); +check("a foreign host with the right port is still refused", await get(`evil.example.com:${port}`), 403); +check("the loopback address is served", await get(`127.0.0.1:${port}`), 200); +check("the loopback name is served", await get(`localhost:${port}`), 200); +check("an IPv6 loopback literal is served", await get(`[::1]:${port}`), 200); +check("another 127.x address is served", await get(`127.0.0.2:${port}`), 200); +// A host that only starts with an allowed name must not pass. +check("a lookalike host is refused", await get("localhost.evil.example.com"), 403); + +server.close(); + +if (failures.length) { + console.log(`host validation: ${failures.length} check(s) FAILED\n`); + for (const f of failures) console.log(` [FAIL] ${f}\n`); + process.exit(1); +} +console.log("host validation: all checks passed"); From eddb3dfd57286484ef484d89c84e32b40f35ef9d Mon Sep 17 00:00:00 2001 From: Korivi Date: Mon, 7 Sep 2026 19:56:02 +0900 Subject: [PATCH 5/6] Write the ignore rules from the CLI, not only from the template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit npm strips files named `.gitignore` out of a published tarball. All four blueprint templates ship one, `files` includes `toolkits`, and none of them survive: `npm pack --dry-run` on framework/cli lists 189 files and zero gitignore entries. So `npm i -g agent-app` followed by `scaffold` produced an app directory with no ignore rules at all, and the next `git add -A && git push` published the agent token, the principal password and the live database. The rules the framework itself depends on are now written by the CLI on both scaffold paths, and the database rule follows the toolkit's declared lifecycle.dataDir rather than assuming `data/` — a pocketbase app gets `pb/pb_data/`. Existing lines are kept and only missing ones appended, so a blueprint's own rules survive and re-running changes nothing. Verified by scaffolding with the template's .gitignore deleted, which is what an npm install looks like: the app still comes out protected. blueprint-react-node's template was also the odd one out — it omitted .superuser and every .a2app path, and narrowed the database to data/db.json while backups are written to .a2app/backups/. Brought in line with the other three. --- framework/cli/src/commands/scaffold.ts | 62 +++++++++++++------ .../blueprint-react-node/template/.gitignore | 15 ++++- 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/framework/cli/src/commands/scaffold.ts b/framework/cli/src/commands/scaffold.ts index ae154cc..147b086 100644 --- a/framework/cli/src/commands/scaffold.ts +++ b/framework/cli/src/commands/scaffold.ts @@ -80,6 +80,7 @@ export async function run(args: string[], app: string): Promise { throw new UsageError(err instanceof Error ? err.message : String(err)); } vendorPaths(tk, dir, allTemplateFiles(tk)); + ensureIgnoreRules(dir, tk.manifest.lifecycle?.dataDir); recordProjectToolkit(dir, tk); systemPaths = tk.manifest.systemPaths; adapterVersion = adapterVersionOf(tk); @@ -246,29 +247,52 @@ function allTemplateFiles(_tk: ResolvedToolkit): string[] { return ["."]; } +/** + * Guarantee the ignore rules that keep runtime artifacts out of a repository. + * + * Blueprints ship their own `.gitignore`, but npm strips dotfiles named + * `.gitignore` out of a published tarball, so a CLI installed with `npm i -g` + * scaffolds an app carrying none at all — and the next `git add -A` publishes + * the agent token, the principal password and the live database. The rules the + * framework depends on are therefore written here, from the CLI, on every path. + * + * Existing lines are kept and only missing ones appended: a blueprint's own + * rules (node_modules, the PocketBase binary, __pycache__) are none of the + * framework's business, and re-running must not churn the file. + */ +function ensureIgnoreRules(dir: string, dataDir?: string): void { + const rules = [ + ".agent-token", + ".principal", + ".superuser", + ".a2app/serve.json", + ".a2app/serve.log", + ".a2app/dev/", + ".a2app/backups/", + ]; + // The database lives wherever the toolkit says it does, so the rule follows + // the declaration rather than assuming "data/". + if (dataDir !== undefined && dataDir.trim() !== "") { + rules.push(dataDir.trim().replace(/[\/]+$/, "") + "/"); + } + const file = join(dir, ".gitignore"); + const existing = existsSync(file) ? readFileSync(file, "utf8") : ""; + const present = new Set(existing.split(/\r?\n/).map((line) => line.trim())); + const missing = rules.filter((rule) => !present.has(rule)); + if (missing.length === 0) return; + const header = [ + "# Written by agent-app: credentials, framework state and the live database", + "# are runtime artifacts and must never be committed.", + ]; + const prefix = existing.trim() === "" ? "" : existing.replace(/\s*$/, "") + "\n\n"; + writeFileSync(file, prefix + [...header, ...missing, ""].join("\n")); +} + /** A stack-free skeleton: just the framework files, so `scaffold` without a * blueprint still yields a conforming artifact shell an agent then fills in. */ function scaffoldMinimal(dir: string, name: string): void { mkdirSync(join(dir, "reference"), { recursive: true }); - // Credentials are runtime artifacts that must never be committed (section 4.1). - // A blueprint ships its own ignore rules; a hand-assembled app has no other - // source for them, and the token is minted into this directory moments later. - writeFileSync( - join(dir, ".gitignore"), - [ - "# Credentials are runtime artifacts: never shipped, exported, or committed.", - ".agent-token", - ".principal", - ".superuser", - "", - "# Framework runtime state: serve records, logs, dev copies, and backups.", - ".a2app/serve.json", - ".a2app/serve.log", - ".a2app/dev/", - ".a2app/backups/", - "", - ].join("\n"), - ); + ensureIgnoreRules(dir); writeFileSync( join(dir, "operations.json"), JSON.stringify({ operations: [] }, null, 2) + "\n", diff --git a/toolkits/blueprint-react-node/template/.gitignore b/toolkits/blueprint-react-node/template/.gitignore index 6ff15a0..4a1ab24 100644 --- a/toolkits/blueprint-react-node/template/.gitignore +++ b/toolkits/blueprint-react-node/template/.gitignore @@ -1,4 +1,15 @@ -node_modules/ -data/db.json +# Credentials are runtime artifacts: never shipped, exported, or committed. .agent-token .principal +.superuser + +# The live database and adapter-owned state (the toolkit's lifecycle dataDir). +data/ + +# Framework runtime state: serve records, logs, dev copies, and backups. +.a2app/serve.json +.a2app/serve.log +.a2app/dev/ +.a2app/backups/ + +node_modules/ From dd80458357a43c62275915e2f35baeb94428ec14 Mon Sep 17 00:00:00 2001 From: Korivi Date: Mon, 7 Sep 2026 19:57:09 +0900 Subject: [PATCH 6/6] Cap the request body in the Python blueprint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `await request.body()` concatenates the whole stream with no limit, and uvicorn imposes none either. It ran before the origin check, the credential check and the rate gate — before routing at all, so the path did not even have to exist: POST /api/junk with a multi-gigabyte body was buffered in full and only then answered 404. adapter-core caps at 5 MiB and says why: "a protocol write is tiny; anything larger is a mistake or an attack, and buffering it whole would let a hostile client OOM the app". The react-node blueprint inherits that; this one had no equivalent. Count bytes as they arrive rather than trusting Content-Length, and answer 413 in the protocol's own envelope. A malformed body now becomes {"__unparsed__": ...} for the guard to reject on its own terms, matching adapter-core, instead of raising out of json.loads into a bare 500. --- .../blueprint-python-fastapi/template/main.py | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/toolkits/blueprint-python-fastapi/template/main.py b/toolkits/blueprint-python-fastapi/template/main.py index 415ca82..85cd457 100644 --- a/toolkits/blueprint-python-fastapi/template/main.py +++ b/toolkits/blueprint-python-fastapi/template/main.py @@ -50,12 +50,56 @@ def _agent_token() -> str: app = FastAPI(title=manifest["name"]) +# A protocol write is tiny; anything larger is a mistake or an attack. Reading +# the body happens before the origin, credential and rate checks — the path does +# not even have to exist — so an unauthenticated caller must never be able to +# make the app buffer an arbitrary amount of memory. adapter-core caps at the +# same size for the same reason (adapters/adapter-core/src/http.ts). +MAX_REQUEST_BODY_BYTES = 5 * 1024 * 1024 + + +async def _read_body(request: Request): + """Parse the request body under a cap. + + Returns ``(body, None)``, or ``(None, reply)`` when the body is too large. + Bytes are counted as they arrive rather than trusting Content-Length, so a + client that lies about the length gains nothing. + """ + size = 0 + chunks = [] + async for chunk in request.stream(): + size += len(chunk) + if size > MAX_REQUEST_BODY_BYTES: + return None, ( + 413, + { + "a2app": True, + "ok": False, + "code": "payload_too_large", + "message": f"Request body exceeds the {MAX_REQUEST_BODY_BYTES}-byte limit.", + "limitBytes": MAX_REQUEST_BODY_BYTES, + }, + ) + chunks.append(chunk) + raw = b"".join(chunks) + if not raw.strip(): + return {}, None + try: + return json.loads(raw), None + except ValueError: + # Hand the guard something it can reject by its own rules, rather than + # raising into a 500 that tells the caller nothing. Same envelope as + # adapter-core's parser. + return {"__unparsed__": raw.decode("utf8", "replace")}, None + + @app.api_route("/api/{path:path}", methods=["GET", "POST", "PATCH", "DELETE"]) async def a2app_route(path: str, request: Request): body = None if request.method in ("POST", "PATCH"): - raw = await request.body() - body = json.loads(raw) if raw else {} + body, too_large = await _read_body(request) + if too_large is not None: + return JSONResponse(status_code=too_large[0], content=too_large[1]) status, payload = adapter.dispatch( request.method, f"/api/{path}", dict(request.headers), body, dict(request.query_params) )