Skip to content

Commit 608bf7a

Browse files
thymikeeclaude
andauthored
Harden the MCP surface: registry rug-pull fix, operator-only credentials/endpoints, device-shell argv gate, declared timeouts (#2023)
* chore(release): keep the version on main distinct from every published version Registry scanners diff the repository's tool surface per version string, so a released number left on main while main keeps changing is indistinguishable from a republished ("rug-pull") version — two scans of the same version see two different tool sets (AS-012). - release:publish now runs release:mark-dev after npm publish, moving package.json and the synchronized server.json to the next patch with a -dev prerelease marker. - release:prepare refuses to publish while the -dev marker is in place, so a forgotten version bump cannot ship a prerelease as latest. - Mark the current tree 0.20.11-dev: main had been sitting on the published 0.20.10 while the tool surface kept changing, which is the live finding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8 * fix(mcp): remove credential inputs from the model-writable tool surface Every MCP tool advertised daemonAuthToken (and the Metro tools bearerToken) as a free-form string the model writes. The model both reads untrusted app UI text and picks tool arguments, so on-screen text steering it to set a token was a prompt-injection exfiltration path. Credentials are operator-owned: - the keys are omitted from every advertised tool schema (MCP and AI SDK, which share listCommandTools()), - an explicit value is refused with env-var guidance instead of being forwarded (the retired-field posture: refuse, never silently drop), - operator-sourced values are untouched — env/config defaults still merge, and the daemon and Metro clients keep their AGENT_DEVICE_DAEMON_AUTH_TOKEN / AGENT_DEVICE_METRO_BEARER_TOKEN fallbacks. CLI flags are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8 * fix(mcp): move operator endpoints and paths off the model-writable surface; declare timeouts Follow-up to the credential removal: daemonBaseUrl and the Metro proxyBaseUrl are the endpoints the env-resolved tokens are SENT to, so a model-writable value redirects the operator's token to an arbitrary server — same exfiltration path, one step removed. stateDir, cwd, iosSimulatorDeviceSet, and the three iosXctest* paths select operator infrastructure, never per-call work. All of them leave the advertised MCP/AI-SDK tool schemas and are refused as explicit input with env/config guidance; operator env/config defaults keep flowing exactly as before (config-backed defaults still merge, and explicit input can no longer override them). Dropping these shared properties also cuts tools/list substantially. Every tool description now also declares its enforced client timeout envelope (90s default, 180s install, unbounded only for the streaming test runner), sourced from the descriptor registry's timeout policy so the declared number cannot drift from the enforced one (answers AS-011, which read the undeclared envelope as "no timeout"). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8 * feat(ci): inventory every dynamic value reaching a device shell adb shell, adb exec-out, and hdc shell join their argv into one string the device's sh evaluates, so any unquoted dynamic element is a potential argv injection — the class of bug the audit found (and fixed) on input text and cmd clipboard set text. Nothing enumerated the surface, so a new call site could regress it silently. scripts/shell-argv is an AST-based gate (oxc-parser, same as di-seams and layering) keeping an exact inventory of every dynamic device-shell argv element, keyed by (file, expression) with counts: 121 values today. A new or grown entry fails CI until the author quotes it through shellQuoteIfNeeded or records it with --update in the same PR, making "a new value now reaches the device shell" a reviewable diff; a stale entry fails the other way so the inventory always matches the code. Wired as the shell-argv gate in the lint lane and registered in the check catalog. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8 * fix(shell-argv): satisfy the fallow audit without suppressions The Compatibility & Provenance lane's fallow audit flagged the new gate: main was an unused export (only the self-run guard consumed it) and four functions sat over the complexity thresholds. Restructure instead of suppressing: the AST walk dispatches through a composite-child-field table, the argv detection is hoisted out of the visitor, drift reporting moves into helpers, and main is no longer exported. Behavior is unchanged — the model tests pass as written and the regenerated inventory is byte-identical (121 values). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8 * fix(test): close the port-reuse race in the unreachable-takeover test Coverage (1) failed once in CI with the takeover notice missing while the response still came from the fresh daemon — the exact signature of the fresh fixture being handed the just-freed ephemeral port: the recorded daemon becomes reachable and reusable (same version and signature), so the takeover path is skipped. Bind the fresh fixture before acquiring and freeing the unreachable port; with no bind after the close, the port can never be reclaimed. Line-neutral so the size-ratchet pin holds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8 * fix(mcp): enforce the advertised tool schema at a real admission boundary P1 (reported by the PR author): hiding operator keys from tools/list did not stop them reaching the command route. The router forwards raw tools/call arguments verbatim and resolveMcpConfigDefaults reads them as CLI flags, so an unadvertised `config`/`remoteConfig` key loaded an arbitrary file whose daemonBaseUrl/daemonAuthToken then flowed to runCommand — a model-writable redirect to an attacker endpoint with the operator's token. Reproduced: {config: <path>} on `snapshot` put both values into the command input. Replace the per-key operator refusal with a deny-by-default admission boundary in the shared executor (the one path both the MCP router and the AI SDK adapter use): every raw input key must appear in the tool's advertised schema, else it is rejected with guidance BEFORE config/env resolution. This closes the config loaders, the operator keys, and any unknown key at once, and makes the advertised additionalProperties:false contract actually enforced. Operator env/config defaults still resolve — they never arrive as tool input. Retired keys (maxSize) are admitted so the command's own reader still answers with migration guidance; they're exposed as metadata.retiredInputKeys for that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8 * revert(ci): remove the check:shell-argv inventory gate The PR author correctly flagged that this gate is an inventory, not a security invariant: --update lets any site self-approve a raw value, and the literal-first array heuristic is blind to indirect argv (a variable-built subcommand, or an argv assembled in a helper). Reproduced: adb(['shell', 'input','text',text]) is inventoried, but const s='shell'; adb([s,'input','text',text]) yields no finding. Shipping it security-framed gives false assurance. Remove it. The sound fix — a typed device-shell execution boundary where a raw string cannot reach adb/hdc shell without being quoted or explicitly marked — is a ~188-site cross-package migration on device execution paths, scoped to a dedicated follow-up PR. The two known-dangerous sites (input text, cmd clipboard set text) already quote through shellQuoteIfNeeded on main, so no regression. This keeps the PR focused on the MCP tool surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 17cfd8c commit 608bf7a

16 files changed

Lines changed: 542 additions & 24 deletions

CHANGELOG.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,37 @@
22

33
## Unreleased
44

5+
- Security (MCP/AI-SDK tool surface): the operator-owned endpoint and path inputs — `daemonBaseUrl`,
6+
the Metro `proxyBaseUrl`, `stateDir`, `cwd`, `iosSimulatorDeviceSet`, `iosXctestrunFile`,
7+
`iosXctestDerivedDataPath`, `iosXctestEnvDir` — follow the credential inputs off the
8+
model-writable tool surface: no longer advertised, refused as explicit input with guidance, and
9+
resolved from env/config only (a model-writable `daemonBaseUrl`/`proxyBaseUrl` would redirect the
10+
env-resolved token to an arbitrary server). Dropping these plus the credential fields shrinks
11+
`tools/list` by roughly half. CLI flags and the SDK client options are unchanged.
12+
- MCP tool descriptions now declare their enforced client timeout envelope (90s default, 180s
13+
install, 300s+ lease allocation, unbounded only for the streaming `test` runner), sourced from
14+
the descriptor registry's timeout policy so the declared number cannot drift from the enforced
15+
one.
16+
- Security (MCP/AI-SDK tool surface): the shared command-tool executor now enforces the advertised
17+
tool schema as an admission boundary — every raw `tools/call` argument must appear in the tool's
18+
advertised (`additionalProperties: false`) schema, or it is refused before config/env resolution.
19+
Hiding a key from `tools/list` alone was insufficient: the router forwards raw arguments verbatim
20+
and the MCP config resolver read `config`/`remoteConfig` as CLI flags, so a model-supplied config
21+
file could load `daemonBaseUrl`/`daemonAuthToken` and redirect the operator's token to an
22+
arbitrary endpoint. Deny-by-default closes that, the operator keys, and any unknown key at once;
23+
operator env/config defaults still resolve (they never arrive as tool input). Retired keys are
24+
still admitted so their migration guidance answers.
25+
- Security (MCP/AI-SDK tool surface): `daemonAuthToken` and the Metro `bearerToken` are no longer
26+
advertised as tool input properties, and an explicit value is refused with guidance instead of
27+
being forwarded. Credentials are operator-owned: set `AGENT_DEVICE_DAEMON_AUTH_TOKEN` (or
28+
`daemonAuthToken` in `~/.agent-device/config.json`) and `AGENT_DEVICE_METRO_BEARER_TOKEN` on the
29+
process serving the tools. The model both reads untrusted app UI text and picks tool arguments,
30+
so a model-writable credential parameter was a prompt-injection exfiltration path. CLI flags
31+
(`--daemon-auth-token`, `--bearer-token`) and env/config resolution are unchanged.
32+
- Release hygiene: after `npm publish`, `release:mark-dev` moves `main` to the next patch with a
33+
`-dev` prerelease marker so the version on `main` never equals a published version (registry
34+
scanners diff the tool surface per version string, and a moving surface under a released number
35+
reads as a republish). `release:prepare` refuses to publish while the `-dev` marker is in place.
536
- Parameterized `fill --record-as` protection is now recording-session-scoped instead of
637
fill-step-scoped (ADR 0017 amendment): a later, unrelated recorded action (`wait`, `is`, `get`) can no
738
longer re-serialize an app-rendered echo of an already-parameterized value into its own result or

CONTRIBUTING.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,17 @@ when a running daemon needs to pick up a new TypeScript build.
7272
`pnpm package:npm` is a release guard, not a routine development command. Use the specific commands
7373
above while iterating.
7474

75+
### The version on main never equals a published version
76+
77+
`release:publish` runs `release:mark-dev` right after `npm publish`, moving `package.json` (and the
78+
synchronized `server.json`) to the next patch with a `-dev` prerelease marker (for example
79+
`0.20.11-dev`). Commit that bump as part of the release. The invariant it protects: MCP registry
80+
scanners diff the repository's tool surface per version string, so a released number left on `main`
81+
while `main` keeps changing is indistinguishable from a republished ("rug-pull") version.
82+
`release:prepare` enforces the inverse direction and refuses to publish while the `-dev` marker is
83+
still in place — set the real release version first (for example `npm version patch`, which strips
84+
the prerelease marker), commit, then publish.
85+
7586
### Released-surface baselines roll forward on publish
7687

7788
Compatibility gates baseline against the last **released tag**, not against `main`, so publishing is

package.json

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "agent-device",
3-
"version": "0.20.10",
3+
"version": "0.20.11-dev",
44
"description": "Mobile app automation and verification for AI coding agents. CLI, MCP server, and typed Node.js API for iOS, Android, HarmonyOS, TV, web, macOS, and Linux.",
55
"mcpName": "io.github.callstack/agent-device",
66
"license": "MIT",
@@ -109,8 +109,9 @@
109109
"build:macos-helper:clean": "node --experimental-strip-types scripts/swift-toolchain-tmpdir.ts swift package --package-path apple/macos-helper clean && pnpm build:macos-helper",
110110
"build:package": "pnpm build && pnpm build:xcuitest:ios && pnpm build:xcuitest:macos && pnpm build:xcuitest:tvos && pnpm build:xcuitest:visionos && pnpm build:macos-helper:clean && pnpm package:apple-runner:npm && pnpm build:android",
111111
"package:npm": "pnpm build:package && pnpm check:package",
112-
"release:prepare": "rm -rf .tmp/release && pnpm check:mcp-metadata && pnpm build:package && pnpm check:package -- --pack-destination .tmp/release",
113-
"release:publish": "pnpm release:prepare && npm publish --ignore-scripts .tmp/release/*.tgz",
112+
"release:prepare": "node scripts/release-mark-dev.mjs --check-release-version && rm -rf .tmp/release && pnpm check:mcp-metadata && pnpm build:package && pnpm check:package -- --pack-destination .tmp/release",
113+
"release:publish": "pnpm release:prepare && npm publish --ignore-scripts .tmp/release/*.tgz && pnpm release:mark-dev",
114+
"release:mark-dev": "node scripts/release-mark-dev.mjs",
114115
"ad": "node bin/agent-device.mjs",
115116
"bench:help-conformance": "node scripts/help-conformance-bench.mjs",
116117
"maestro:conformance": "node --experimental-strip-types scripts/node-test-tmpdir.ts --experimental-strip-types --test scripts/maestro-conformance/format-generated-json.test.mjs packages/maestro/test/conformance/verify.test.ts packages/maestro/test/conformance/differential/engine-process.test.ts packages/maestro/test/conformance/differential/report-output.test.ts packages/maestro/test/conformance/differential/run.test.ts packages/maestro/test/conformance/differential/invariants.test.ts",

scripts/__tests__/test-file-size-ratchet.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ const PINNED_TEST_FILE_LINES: Readonly<Record<string, number>> = Object.freeze({
5050
'src/__tests__/cli-config.test.ts': 1282,
5151
'src/daemon/handlers/__tests__/find.test.ts': 1202,
5252
'src/platforms/apple/core/__tests__/perf.test.ts': 1222,
53-
'src/mcp/__tests__/command-tools.test.ts': 1218,
53+
'src/mcp/__tests__/command-tools.test.ts': 1216,
5454
'src/daemon/handlers/__tests__/session-replay-divergence.test.ts': 1137,
5555
'src/platforms/apple/core/__tests__/apps.test.ts': 1210,
5656
'src/daemon/handlers/__tests__/session-replay-repair-transaction.test.ts': 1208,

scripts/release-mark-dev.mjs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { spawnSync } from 'node:child_process';
2+
import fs from 'node:fs';
3+
import path from 'node:path';
4+
import process from 'node:process';
5+
6+
// The version on main must never equal a published version: registry scanners
7+
// diff the repository's tool surface per version string, so a released number
8+
// left in place while main keeps moving reads as a rug-pull republish
9+
// (AS-012). `release:publish` runs this right after `npm publish` to move
10+
// main to the next patch with a `-dev` prerelease marker; `--check-release-version`
11+
// is the inverse guard in `release:prepare`, refusing to publish a `-dev`
12+
// version because the maintainer has not set the release version yet.
13+
const root = process.cwd();
14+
const checkReleaseVersion = process.argv.includes('--check-release-version');
15+
const packagePath = path.join(root, 'package.json');
16+
17+
const raw = fs.readFileSync(packagePath, 'utf8');
18+
const pkg = JSON.parse(raw);
19+
const version = pkg.version;
20+
21+
if (typeof version !== 'string' || version.length === 0) {
22+
fail('package.json must define version.');
23+
}
24+
25+
if (checkReleaseVersion) {
26+
if (version.includes('-')) {
27+
fail(
28+
`package.json version ${version} is a prerelease marker. Set the release version first ` +
29+
'(e.g. `npm version patch`), commit, then publish.',
30+
);
31+
}
32+
process.exit(0);
33+
}
34+
35+
if (version.includes('-')) {
36+
process.stdout.write(`Version ${version} already carries a prerelease marker; nothing to do.\n`);
37+
process.exit(0);
38+
}
39+
40+
const match = /^(\d+)\.(\d+)\.(\d+)$/.exec(version);
41+
if (!match) {
42+
fail(`Unsupported version format: ${version}`);
43+
}
44+
const nextVersion = `${match[1]}.${match[2]}.${Number(match[3]) + 1}-dev`;
45+
46+
const versionField = `"version": "${version}"`;
47+
if (raw.split(versionField).length !== 2) {
48+
fail(`Expected exactly one ${versionField} in package.json.`);
49+
}
50+
fs.writeFileSync(packagePath, raw.replace(versionField, `"version": "${nextVersion}"`));
51+
52+
const sync = spawnSync(process.execPath, [path.join(root, 'scripts', 'sync-mcp-metadata.mjs')], {
53+
stdio: 'inherit',
54+
});
55+
if (sync.status !== 0) {
56+
fail('sync-mcp-metadata failed after the version bump.');
57+
}
58+
59+
process.stdout.write(
60+
`Marked main as unreleased: ${version} -> ${nextVersion} (package.json + server.json).\n` +
61+
`Commit and push this so the version on main never equals the published ${version}.\n`,
62+
);
63+
64+
function fail(message) {
65+
process.stderr.write(`${message}\n`);
66+
process.exit(1);
67+
}

server.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77
"url": "https://github.com/callstack/agent-device",
88
"source": "github"
99
},
10-
"version": "0.20.10",
10+
"version": "0.20.11-dev",
1111
"packages": [
1212
{
1313
"registryType": "npm",
1414
"identifier": "agent-device",
15-
"version": "0.20.10",
15+
"version": "0.20.11-dev",
1616
"transport": {
1717
"type": "stdio"
1818
}

src/__tests__/npm-package-scripts.test.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,28 @@ test('the npm package build covers every package-owned output before verificatio
7373
test('release publishing uploads the tarball that passed the package gate', () => {
7474
assert.equal(
7575
script('release:prepare'),
76-
'rm -rf .tmp/release && pnpm check:mcp-metadata && pnpm build:package && pnpm check:package -- --pack-destination .tmp/release',
76+
'node scripts/release-mark-dev.mjs --check-release-version && rm -rf .tmp/release && pnpm check:mcp-metadata && pnpm build:package && pnpm check:package -- --pack-destination .tmp/release',
7777
);
7878
assert.equal(
7979
script('release:publish'),
80-
'pnpm release:prepare && npm publish --ignore-scripts .tmp/release/*.tgz',
80+
'pnpm release:prepare && npm publish --ignore-scripts .tmp/release/*.tgz && pnpm release:mark-dev',
8181
);
8282
assert.doesNotMatch(script('release:publish'), /prepack|package:npm/);
8383
});
8484

85+
// AS-012: registry scanners diff the repository's tool surface per version string, so the
86+
// version on main must never equal a published version. Publishing marks main as unreleased
87+
// (`-dev` prerelease on the next patch) right after the upload, and preparation refuses to
88+
// publish while that marker is still in place.
89+
test('release publishing moves main off the released version', () => {
90+
assert.match(
91+
script('release:prepare'),
92+
/^node scripts\/release-mark-dev\.mjs --check-release-version && /,
93+
);
94+
assert.match(script('release:publish'), / && pnpm release:mark-dev$/);
95+
assert.equal(script('release:mark-dev'), 'node scripts/release-mark-dev.mjs');
96+
});
97+
8598
test('the package checker can retain the tarball it verifies for publishing', () => {
8699
const gate = fs.readFileSync(path.join(repoRoot, 'scripts', 'check-package.ts'), 'utf8');
87100
assert.match(gate, /--pack-destination/);

src/commands/command-contract.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ export type CommandMetadata<Name extends string, Input> = {
2727
mcpDetail?: string;
2828
inputSchema: JsonSchema;
2929
readInput: (input: unknown) => Input;
30+
/**
31+
* Released input keys the command removed. Excluded from `inputSchema` (not
32+
* advertised), but still recognized: `readInput` throws migration guidance
33+
* when one is supplied. The MCP admission boundary reads this so it lets a
34+
* retired key reach that message instead of rejecting it as unknown.
35+
*/
36+
retiredInputKeys?: readonly string[];
3037
};
3138

3239
export type ExecutableCommandContract<Name extends string, Input, Result> = CommandMetadata<

src/commands/command-input.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,11 @@ function requiredFieldNames(fields: CommandFieldMap): string[] {
568568
return Object.entries(fields).flatMap(([key, field]) => (field.required ? [key] : []));
569569
}
570570

571+
/** Names of the retired fields — declared for migration guidance, absent from the schema. */
572+
export function retiredFieldNames(fields: CommandFieldMap): string[] {
573+
return Object.entries(fields).flatMap(([key, field]) => (field.retired ? [key] : []));
574+
}
575+
571576
function optionalRecord(
572577
record: Record<string, unknown>,
573578
key: string,

src/commands/field-command-contract.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { defineCommandMetadata } from './command-contract.ts';
2-
import { fieldsInputSchema, readFieldInput, type CommandFieldMap } from './command-input.ts';
2+
import {
3+
fieldsInputSchema,
4+
readFieldInput,
5+
retiredFieldNames,
6+
type CommandFieldMap,
7+
} from './command-input.ts';
38

49
export function defineFieldCommandMetadata<
510
const TName extends string,
@@ -10,5 +15,6 @@ export function defineFieldCommandMetadata<
1015
description,
1116
inputSchema: fieldsInputSchema(fields),
1217
readInput: (input) => readFieldInput(input, fields),
18+
retiredInputKeys: retiredFieldNames(fields),
1319
});
1420
}

0 commit comments

Comments
 (0)