Korivi security v1

adapters/adapter-core/package.json

@@ -27,6 +27,6 @@
  "scripts": {
  "build": "tsc -p tsconfig.json",
  "typecheck": "tsc -p tsconfig.json --noEmit",
- "test": "tsc -p tsconfig.json && node test/static.test.mjs"
+ "test": "tsc -p tsconfig.json && node test/static.test.mjs && node test/host.test.mjs"
  }
 }

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());
 
@@ -411,6 +421,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
@@ -592,7 +621,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<string, unknown> = {};
+    // 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<string, unknown> = Object.create(null) as Record<string, unknown>;
     for (const k of def.writeAllow ?? []) allow[k] = true;
     const violations = validate(def.fields, body, { allow, requireRequired: req.method === "POST" });
     if (violations.length) {
@@ -884,6 +916,13 @@ export function createA2App(binding: Binding, config: A2AppConfig): A2App {
   async function handle(req: A2AppRequest): Promise<A2AppReply | null> {
     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") {
       // The update watcher polls this every few seconds anyway, so its "I am

adapters/adapter-core/src/types.ts

@@ -238,6 +238,9 @@ export interface A2AppConfig {
   conventions?: Record<string, unknown>;
   /** 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. */

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");

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;
 }
 

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<typeof createA2AppServer> {
   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;
 }

adapters/rules/src/index.ts

@@ -196,8 +196,16 @@ export function isDayKeyField(field: Pick<NormalizedField, "type" | "max" | "nam
   return /^(due|day|date)$|_(date|day)$/i.test(field.name);
 }
 
+/**
+ * Field lookup keyed by name. The map has a NULL prototype because its keys come
+ * from the request body: on a plain object, `map["constructor"]` resolves up the
+ * prototype chain and returns a truthy value, so an undeclared field named after
+ * any Object.prototype member reads as declared. It then passes the unknown-field
+ * check and every check after it — the one guarantee this module exists to make,
+ * silently voided by the field's name.
+ */
 function fieldMap(fields: NormalizedField[]): Record<string, NormalizedField> {
-  const m: Record<string, NormalizedField> = {};
+  const m: Record<string, NormalizedField> = Object.create(null) as Record<string, NormalizedField>;
   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<string, unknown> | Record<string, NormalizedField>, 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.

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

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 && node test/manifest-merge.test.mjs",
+    "test": "tsc -p tsconfig.json && node test/copy-tree.test.mjs && node test/manifest-merge.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",

framework/cli/src/commands/scaffold.ts

@@ -80,6 +80,7 @@ export async function run(args: string[], app: string): Promise<number> {
         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);
@@ -259,29 +260,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",

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;
   }