Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/__tests__/seats-package.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('packages/seats', () => {
});

test('the CLI provider proxy is the package module', () => {
expect(Object.keys(cliProxy).sort()).toEqual(['redactProviderMaterial', 'resolveProviderKeyFile', 'startProviderProxy']);
expect(Object.keys(cliProxy).sort()).toEqual(['PROVIDER_WIRES', 'redactProviderMaterial', 'resolveProviderKeyFile', 'startProviderProxy']);
for (const name of Object.keys(cliProxy)) expect(cliProxy[name]).toBe(seatsProxy[name]);
});

Expand Down
4 changes: 2 additions & 2 deletions packages/seats/docs/record-v1.md
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,8 @@ built in Go runs its tests with `-race` in CI.

| kind | data | default actor |
|---|---|---|
| `credential.granted` | `{names: [NAME]}` | `system` |
| `credential.denied` | `{name, reason}`, where `reason` is `not_granted`, `unresolved` or `too_short` (a value under 8 characters) | `system` |
| `credential.granted` | `{names: [NAME], injected?: [NAME]}`. `injected` names the credentials a loopback provider proxy holds and adds outbound; the run's env has only that proxy's token under those names. The other names are held by the run | `system` |
| `credential.denied` | `{name, reason}`, where `reason` is `not_granted`, `unresolved`, `too_short` (a value under 8 characters) or `no_upstream` (marked `inject` with no provider named for it) | `system` |

These cover the environment credentials a run is started with. Values never
appear here: `NAME` is an environment variable name matching
Expand Down
8 changes: 7 additions & 1 deletion packages/seats/src/record/format.js
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,13 @@ const CHECKS = {
optionalArgsDigest(data, 'tool.call');
return data;
},
'credential.granted': (data) => ({ ...data, names: envNames(data.names ?? [], 'credential.granted.names') }),
'credential.granted': (data) => {
const names = envNames(data.names ?? [], 'credential.granted.names');
if (data.injected === undefined) return { ...data, names };
const injected = envNames(data.injected, 'credential.granted.injected');
if (!injected.every((name) => names.includes(name))) fail('credential.granted.injected must be a subset of names');
return { ...data, names, injected };
},
'credential.denied': (data) => {
envNames([data.name], 'credential.denied.name');
return data;
Expand Down
4 changes: 2 additions & 2 deletions packages/seats/src/trace/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export const EVENT_TYPES = Object.freeze([
'usage', // { inputTokens?, outputTokens?, totalTokens?, cost? } null-not-zero when unknown
'liveness.stalled', // { idleMs, windowMs }
'liveness.recovery', // { action: 'cancel'|'kill'|'retry', attempt }
'credential.granted', // { names: [...] } names only, never values
'credential.denied', // { name, reason?: 'not_granted'|'unresolved'|'too_short' }
'credential.granted', // { names: [...], injected?: [...] } names only, never values
'credential.denied', // { name, reason?: 'not_granted'|'unresolved'|'too_short'|'no_upstream' }
'process.exited', // { code, signal }
// Record kinds with no earlier trace name (docs/record-v1.md has the full data).
'task.claimed', // { leaseUntil? } mirrors the board's claim; the board stays the source of truth
Expand Down
54 changes: 43 additions & 11 deletions packages/seats/src/valet/env.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { traceOf } from '../trace/events.js';
import { runFailure } from '../failures.js';
import { MIN_CREDENTIAL_LENGTH } from '../upstream-refusal.js';
import { validateGrant } from './grant.js';
import { startProviderProxy } from '../wire/provider-proxy.js';

// Same spirit as the goose adapter's SAFE_ENV: locale, terminal and TLS trust
// only. USER/LOGNAME are left out on purpose; they name the account whose
Expand Down Expand Up @@ -104,8 +105,8 @@ const grantIssued = (meta) => {
};

/**
* prepareRunEnv({ grant, baseEnv, resolveSecret, root?, require?, trace?, ctx?, grantMeta? })
* → { env, home, cleanup, granted: [names], grantId? }
* prepareRunEnv({ grant, baseEnv, resolveSecret, root?, require?, trace?, ctx?, grantMeta?, upstreams? })
* → { env, home, cleanup, granted: [names], injected: { NAME: { baseURL, wire } }, grantId? }
*
* `require` lists credential names the adapter cannot run without; one the
* grant does not name fails the run as credential_denied before any spawn.
Expand All @@ -114,14 +115,25 @@ const grantIssued = (meta) => {
* silently lacks its key fails later and less legibly, and a value that short
* would slip past the refusal-text reader's redaction.
*
* A credential the grant marks `inject: true` never reaches the env. The host
* names its provider in `upstreams[NAME]` ({ baseURL, wire }, wire defaulting
* to openai-chat); the run gets a loopback provider proxy holding the value in
* memory, env[NAME] becomes that proxy's token, and `injected[NAME].baseURL`
* is where the host points the CLI. cleanup() closes the proxies. One with no
* upstream fails as credential_denied (no_upstream) before it is resolved.
* The proxy listens on 127.0.0.1, so a run without the host's network
* (bwrap --unshare-net) cannot reach it yet.
*
* Events go to `trace`, else ctx.trace: grant.issued (only with grantMeta,
* first), then credential.granted {names} or credential.denied {name, reason}.
* first), then credential.granted {names, injected?} or credential.denied
* {name, reason}. `injected` lists the names held by a proxy; the rest of
* `names` are held by the run.
* Values never enter an event or a message, and a hook that offers
* redact(values) is handed the resolved values so the record can scrub them
* out of agent text too.
* redact(values) is handed the resolved values and each proxy token, so the
* record can scrub them out of agent text too.
*/
export const prepareRunEnv = async ({
grant, baseEnv = {}, resolveSecret, root, require: required = [], trace, ctx, grantMeta,
grant, baseEnv = {}, resolveSecret, root, require: required = [], trace, ctx, grantMeta, upstreams = {},
} = {}) => {
const valid = validateGrant(grant);
const hook = trace && typeof trace.emit === 'function' ? trace : traceOf(ctx);
Expand All @@ -132,6 +144,7 @@ export const prepareRunEnv = async ({
not_granted: 'is not in the run grant',
unresolved: 'could not be resolved',
too_short: `resolved to a value shorter than ${MIN_CREDENTIAL_LENGTH} characters`,
no_upstream: 'is marked inject, but the host named no upstream for it',
};
const deny = (name, reason, values) => {
hook.emit('credential.denied', { name, reason });
Expand All @@ -143,11 +156,18 @@ export const prepareRunEnv = async ({
if (missing) throw deny(missing, 'not_granted', []);

const { home, dirs } = await makeHome(root);
const cleanup = () => rm(home, { recursive: true, force: true });
const proxies = [];
const cleanup = async () => {
await Promise.all(proxies.splice(0).map((proxy) => proxy.close()));
await rm(home, { recursive: true, force: true });
};
const env = { ...pickBaseEnv(baseEnv, realHomes), HOME: home, ...dirs };
const values = [];
const injected = {};
try {
for (const spec of valid.credentials) {
const upstream = spec.inject ? upstreams[spec.name] : undefined;
if (spec.inject && !upstream) throw deny(spec.name, 'no_upstream', values);
let value;
// A resolver's error text may quote the secret it failed on; drop it.
try { value = await resolveOne(spec, { baseEnv, resolveSecret }); } catch { value = undefined; }
Expand All @@ -156,14 +176,26 @@ export const prepareRunEnv = async ({
// refusal-text reader would not redact it. Refuse it here, before spawn.
if (value.length < MIN_CREDENTIAL_LENGTH) throw deny(spec.name, 'too_short', values);
values.push(value);
env[spec.name] = value;
if (upstream) {
const wire = upstream.wire ?? 'openai-chat';
const proxy = await startProviderProxy({
provider: { baseURL: upstream.baseURL }, keyMaterial: value, wire, adapterLabel: 'Valet',
});
proxies.push(proxy);
env[spec.name] = proxy.token;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
injected[spec.name] = { baseURL: proxy.baseURL, wire };
} else {
env[spec.name] = value;
}
}
} catch (error) {
await cleanup();
throw error;
}
if (typeof hook.redact === 'function') hook.redact(values);
// A proxy token authorizes provider calls until cleanup, so the record scrubs it too.
if (typeof hook.redact === 'function') hook.redact([...values, ...proxies.map((proxy) => proxy.token)]);
const names = valid.credentials.map((c) => c.name);
hook.emit('credential.granted', { names });
return { env, home, cleanup, granted: names, ...(issued ? { grantId: issued.grantId } : {}) };
const proxied = Object.keys(injected);
hook.emit('credential.granted', proxied.length ? { names, injected: proxied } : { names });
return { env, home, cleanup, granted: names, injected, ...(issued ? { grantId: issued.grantId } : {}) };
};
13 changes: 9 additions & 4 deletions packages/seats/src/valet/grant.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,18 @@
* and which tool calls it may make without asking, only after asking, or never.
*
* {
* credentials: [{ name: 'OPENAI_API_KEY', from: 'env'|'file'|'resolver', ref? }],
* credentials: [{ name: 'OPENAI_API_KEY', from: 'env'|'file'|'resolver', ref?, inject? }],
* tools: { allow: [pattern], ask: [pattern], deny: [pattern] },
* }
*
* `from: 'env'` copies baseEnv[ref ?? name]; `from: 'file'` reads the absolute
* path in `ref`; `from: 'resolver'` asks the host's resolveSecret(name, spec),
* with `ref` as an opaque hint for it.
*
* `inject: true` keeps the value out of the run: its env gets a per-run proxy
* token under the name instead, and a loopback provider proxy adds the real
* key to outbound requests. Default false: the run holds the value.
*
* A tool pattern is `<kind>` or `<kind>:<title glob>`, where kind is one of
* ACP's ToolKind values or `*`. `execute` matches every execute call;
* `execute:git status*` matches execute calls whose title starts that way;
Expand Down Expand Up @@ -61,8 +65,8 @@ const parsePattern = (pattern, where) => {
const validateCredential = (credential, index, seen) => {
const where = `credentials[${index}]`;
if (!isPlainObject(credential)) fail(`${where} must be an object`);
refuseUnknownKeys(credential, ['name', 'from', 'ref'], `${where}.`);
const { name, from, ref } = credential;
refuseUnknownKeys(credential, ['name', 'from', 'ref', 'inject'], `${where}.`);
const { name, from, ref, inject } = credential;
if (typeof name !== 'string' || !ENV_NAME.test(name)) fail(`${where}.name must be an ENV_NAME (A-Z, 0-9, _)`);
if (RESERVED_NAME.test(name)) fail(`${where}.name ${name} is reserved for the run env`);
if (seen.has(name)) fail(`${where}.name ${name} is granted twice`);
Expand All @@ -71,7 +75,8 @@ const validateCredential = (credential, index, seen) => {
if (ref !== undefined && (typeof ref !== 'string' || !ref)) fail(`${where}.ref must be a non-empty string`);
if (from === 'env' && ref !== undefined && !ENV_NAME.test(ref)) fail(`${where}.ref must be an ENV_NAME for from: env`);
if (from === 'file' && (ref === undefined || !ref.startsWith('/'))) fail(`${where}.ref must be an absolute path for from: file`);
return Object.freeze(ref === undefined ? { name, from } : { name, from, ref });
if (inject !== undefined && typeof inject !== 'boolean') fail(`${where}.inject must be true or false`);
return Object.freeze({ name, from, ...(ref === undefined ? {} : { ref }), ...(inject ? { inject } : {}) });
};

/** Validate and freeze a grant. Throws on the first problem; never repairs one. */
Expand Down
87 changes: 60 additions & 27 deletions packages/seats/src/wire/provider-proxy.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Transform } from 'node:stream';
import { StringDecoder } from 'node:string_decoder';
/** The per-spawn loopback proxy shared by OpenAI-compatible CLI adapters. */
/** The per-spawn loopback proxy that holds a provider key for a CLI (OpenAI chat, Anthropic Messages, OpenAI Responses). */
import { randomBytes, timingSafeEqual } from 'crypto';
import { constants as fsConstants, realpathSync } from 'fs';
import { createServer, request as httpRequest } from 'http';
Expand Down Expand Up @@ -167,11 +167,30 @@ const readProviderKey = async (keyFile, adapterLabel) => {
}
};

// The routes a CLI on each wire may call, relative to the configured base
// path, and the header the real key goes out in. Anything else is refused.
const WIRES = Object.freeze({
'openai-chat': { routes: [['POST', 'chat/completions'], ['GET', 'models']], auth: 'bearer' },
'anthropic-messages': {
routes: [['POST', 'v1/messages'], ['POST', 'v1/messages/count_tokens'], ['GET', 'v1/models']],
auth: 'x-api-key',
},
'openai-responses': { routes: [['POST', 'responses'], ['GET', 'models']], auth: 'bearer' },
});
export const PROVIDER_WIRES = Object.freeze(Object.keys(WIRES));

const presents = (value, expected) => {
const supplied = Buffer.from(typeof value === 'string' ? value : '');
return supplied.length === expected.length && timingSafeEqual(supplied, expected);
};

export const startProviderProxy = async ({
provider, keyFile, keyMaterial, tokenFile, socketPath, isolatedPort, adapterLabel = 'OpenCode',
provider, keyFile, keyMaterial, tokenFile, socketPath, isolatedPort, adapterLabel = 'OpenCode', wire = 'openai-chat',
// When set, every completion must request this model and be served by it.
pinnedModel,
}) => {
const wireSpec = WIRES[wire];
if (!wireSpec) throw new Error(`${adapterLabel} provider wire must be one of ${PROVIDER_WIRES.join(', ')}`);
if (pinnedModel !== undefined && (typeof pinnedModel !== 'string' || !pinnedModel.trim())) {
throw new Error(`${adapterLabel} pinnedModel must be a non-empty string`);
}
Expand All @@ -189,8 +208,13 @@ export const startProviderProxy = async ({
const upstreamKey = keyMaterial === undefined ? await readProviderKey(keyFile, adapterLabel) : keyMaterial;
if (typeof upstreamKey !== 'string' || !upstreamKey || /[\r\n]/.test(upstreamKey)) throw new Error('Invalid provider key');
const proxyToken = randomBytes(32).toString('base64url');
await writeFile(tokenFile, proxyToken, { encoding: 'utf8', mode: 0o600 });
await chmod(tokenFile, 0o600);
// A caller that hands the token to its child in memory needs no file.
if (tokenFile) {
await writeFile(tokenFile, proxyToken, { encoding: 'utf8', mode: 0o600 });
await chmod(tokenFile, 0o600);
}
const bearerToken = Buffer.from(`Bearer ${proxyToken}`);
const bareToken = Buffer.from(proxyToken);

const logRefusedRequest = (method, requestPath) => {
let normalizedPath = '/';
Expand All @@ -204,29 +228,34 @@ export const startProviderProxy = async ({
process.stderr.write(`[${adapterLabel.toLowerCase()}] provider proxy refused ${safeMethod} ${normalizedPath}\n`);
};

// A refusal answers before the request body has been read. A keep-alive
// client that reuses that socket can have its next request reset, so every
// refusal on every wire also closes the connection.
const refuse = (incoming, outgoing, status, headers = {}, body = undefined) => {
incoming.resume();
outgoing.writeHead(status, { ...headers, connection: 'close' });
outgoing.end(body);
};

const server = createServer((incoming, outgoing) => {
const supplied = Buffer.from(incoming.headers.authorization || '');
const expected = Buffer.from(`Bearer ${proxyToken}`);
if (supplied.length !== expected.length || !timingSafeEqual(supplied, expected)) {
incoming.resume();
outgoing.writeHead(401).end();
// Anthropic clients send their key as x-api-key, the others as a Bearer.
if (!presents(incoming.headers.authorization, bearerToken)
&& !presents(incoming.headers['x-api-key'], bareToken)) {
refuse(incoming, outgoing, 401);
return;
}
if (!['GET', 'POST'].includes(incoming.method || '')) {
incoming.resume();
outgoing.writeHead(405).end();
refuse(incoming, outgoing, 405);
return;
}
const rejectRoute = (path) => {
logRefusedRequest(incoming.method, path);
incoming.resume();
outgoing.writeHead(403, { 'content-type': 'text/plain; charset=utf-8' });
outgoing.end(`Unsupported provider request: ${incoming.method || 'UNKNOWN'} ${path}`);
refuse(incoming, outgoing, 403, { 'content-type': 'text/plain; charset=utf-8' },
`Unsupported provider request: ${incoming.method || 'UNKNOWN'} ${path}`);
};
const rejectBadRequest = () => {
logRefusedRequest(incoming.method, incoming.url);
incoming.resume();
outgoing.writeHead(400).end();
refuse(incoming, outgoing, 400);
};

let target;
Expand All @@ -241,8 +270,8 @@ export const startProviderProxy = async ({
return;
}
// The CLI owns this local request path, so never use it as an outbound
// URL. The OpenAI-compatible chat provider needs only these fixed routes;
// parse the incoming URL against a loopback sentinel, then select a
// URL. Each wire needs only the fixed routes in WIRES; parse the
// incoming URL against a loopback sentinel, then select a
// constant endpoint after matching the configured base path exactly.
// This keeps a CLI request from turning the proxy into an arbitrary
// path/URL fetcher (including paths on a local provider host).
Expand All @@ -252,17 +281,13 @@ export const startProviderProxy = async ({
return;
}
const configuredPath = basePath === '/' ? '' : basePath;
let endpoint;
if (incoming.method === 'POST'
&& requested.pathname === `${configuredPath}/chat/completions`) {
endpoint = 'chat/completions';
} else if (incoming.method === 'GET'
&& requested.pathname === `${configuredPath}/models`) {
endpoint = 'models';
} else {
const route = wireSpec.routes.find(([method, path]) => incoming.method === method
&& requested.pathname === `${configuredPath}/${path}`);
if (!route) {
rejectRoute(requested.pathname);
return;
}
const endpoint = route[1];
target = {
path: `${configuredPath}/${endpoint}`,
protocol: upstream.protocol,
Expand All @@ -281,7 +306,8 @@ export const startProviderProxy = async ({
'proxy-authorization', 'te', 'trailer', 'transfer-encoding', 'upgrade',
'x-api-key',
].includes(name.toLowerCase())));
headers.authorization = `Bearer ${upstreamKey}`;
if (wireSpec.auth === 'x-api-key') headers['x-api-key'] = upstreamKey;
else headers.authorization = `Bearer ${upstreamKey}`;
headers.host = target.host;
headers['accept-encoding'] = 'identity';
const pinned = pinnedModel !== undefined && target.path.endsWith('/chat/completions');
Expand Down Expand Up @@ -378,6 +404,12 @@ export const startProviderProxy = async ({
});
});

// A WebSocket client (codex on the Responses wire) falls back to HTTP SSE on
// 426. Nothing is relayed over an upgrade, so no token check is needed first.
server.on('upgrade', (_incoming, socket) => {
socket.end('HTTP/1.1 426 Upgrade Required\r\nconnection: close\r\ncontent-length: 0\r\n\r\n');
});

await new Promise((resolve, reject) => {
server.once('error', reject);
const started = () => {
Expand All @@ -399,6 +431,7 @@ export const startProviderProxy = async ({
const proxyBaseURL = new URL(upstream.pathname, `http://127.0.0.1:${port}`).toString();
return {
baseURL: proxyBaseURL,
token: proxyToken,
tokenFile,
socketPath,
port,
Expand Down
Loading
Loading