feat: AIGRO-4919 - Invoking allowed commands from outside the cli#1598
feat: AIGRO-4919 - Invoking allowed commands from outside the cli#1598mwritter wants to merge 11 commits into
Conversation
📦 Bundle Stats —
|
| Metric | Value | vs main (7332e1f) |
|---|---|---|
| Internal (raw) | 2.2 KB | - |
| Internal (gzip) | 838 B | - |
| Bundled (raw) | 11.20 MB | +1.1 KB, +0.0% |
| Bundled (gzip) | 2.11 MB | +405 B, +0.0% |
| Import time | 758ms | -3ms, -0.4% |
bin:sanity
| Metric | Value | vs main (7332e1f) |
|---|---|---|
| Internal (raw) | 782 B | - |
| Internal (gzip) | 423 B | - |
| Bundled (raw) | 9.90 MB | - |
| Bundled (gzip) | 1.78 MB | - |
| Import time | 1.79s | -15ms, -0.8% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @sanity/cli-core
Compared against main (7332e1f6)
| Metric | Value | vs main (7332e1f) |
|---|---|---|
| Internal (raw) | 118.7 KB | +4.6 KB, +4.0% |
| Internal (gzip) | 30.5 KB | +1.3 KB, +4.4% |
| Bundled (raw) | 21.77 MB | +3.8 KB, +0.0% |
| Bundled (gzip) | 3.47 MB | +793 B, +0.0% |
| Import time | 664ms | -5ms, -0.7% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @sanity/cli-build
Compared against main (7332e1f6)
@sanity/cli-build/_internal/build
| Metric | Value | vs main (7332e1f) |
|---|---|---|
| Internal (raw) | 113.2 KB | - |
| Internal (gzip) | 28.7 KB | - |
| Bundled (raw) | 17.77 MB | +1.4 KB, +0.0% |
| Bundled (gzip) | 3.56 MB | +611 B, +0.0% |
| Import time | 979ms | -3ms, -0.3% |
@sanity/cli-build/_internal/env
| Metric | Value | vs main (7332e1f) |
|---|---|---|
| Internal (raw) | 1.8 KB | - |
| Internal (gzip) | 644 B | - |
| Bundled (raw) | 1.31 MB | - |
| Bundled (gzip) | 333.8 KB | - |
| Import time | 121ms | -1ms, -0.9% |
@sanity/cli-build/_internal/extract
| Metric | Value | vs main (7332e1f) |
|---|---|---|
| Internal (raw) | 8.6 KB | - |
| Internal (gzip) | 2.7 KB | - |
| Bundled (raw) | 155.0 KB | - |
| Bundled (gzip) | 39.5 KB | - |
| Import time | 208ms | -3ms, -1.2% |
🗺️ ./_internal/env · ./_internal/extract · @sanity/cli-build:./_internal/build treemap too large to embed · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — create-sanity
Compared against main (7332e1f6)
| Metric | Value | vs main (7332e1f) |
|---|---|---|
| Internal (raw) | 908 B | - |
| Internal (gzip) | 483 B | - |
| Bundled (raw) | 931 B | - |
| Bundled (gzip) | 491 B | - |
| Import time | ❌ ChildProcess denied: node | - |
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
Coverage Delta
Comparing 13 changed files against main @ Overall Coverage
|
| * @throws When the input contains an unterminated quote. | ||
| * @internal | ||
| */ | ||
| export function tokenizeCliArgs(input: string): string[] { |
There was a problem hiding this comment.
IMO, rather than keeping this in the CLI project, the MCP server should handle parsing the args. this keeps the bundle size smaller for the CLI
| ['cors add', CorsAdd], | ||
| ['cors delete', CorsDelete], | ||
| ['cors list', CorsList], | ||
| ['projects list', ProjectsList], |
There was a problem hiding this comment.
is there a way we can run commands programmatically without needing to define them all here? we want to specify the allowed commands in the MCP server, not the CLI.
There was a problem hiding this comment.
🤔 These are 'invoke-able' commands meant to be called from outside the cli so they are run programmatically. The agent would call the tool with an 'args' string that mimics the actual cli command.
runSanityCli({
args: "cors list --projectId asdfjkl",
token: "mcp-user-token" // extra.authInfo.token
})IMO I actually think it would be the clis responsability to tell the MCP what it should or shouldn't be able to call 🤷♂️ - The cli and anyone working it would know when a breaking change is introduced (ie add a file system call to an already allowed command) and it would be their responsibility to make it work with mcp commands.
So maybe we need to distinguish mcp commands better? Right now we export invokeSanityCli which could technically be used by any project that wants to piggy back off of the cli functionality but maybe we can pass a source and the mcp could have its own dedicated command list (policy)?
This is also why the 'tokenizer' I think should live in the cli as well, because if another source wants to invoke a command it will have to create its own 'tokenizer' as well 🤷♂️
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ab297aa. Configure here.
| function loadCliCommandConfig(): Promise<Config> { | ||
| // Resolves to the package root from both src/exports (dev) and dist/exports (built) | ||
| return Config.load(fileURLToPath(new URL('../..', import.meta.url))) | ||
| } |
There was a problem hiding this comment.
Wrong package root for Config.load
High Severity
loadCliCommandConfig resolves ../.. from invokeSanityCli/index, which lands on src/ or dist/ rather than the package root. The comment assumes a file under exports/, but this module sits one level deeper, so default invocations (no config override) point oclif at a directory without package.json / oclif.config.js.
Reviewed by Cursor Bugbot for commit ab297aa. Configure here.
| } | ||
| } finally { | ||
| process.exitCode = previousExitCode | ||
| } |
There was a problem hiding this comment.
Exit code race under concurrency
Medium Severity
Saving and restoring process.exitCode around each run is not safe under concurrent invocations. One call can observe another’s temporary exit code and later “restore” that non-zero value onto the host process after the other call has already cleaned up.
Reviewed by Cursor Bugbot for commit ab297aa. Configure here.
2 participants
Screenshots - Locally tested linking
clichanges tomellonand runningpnpm inspectfrommellonNote
Medium Risk
Expands the attack surface for remote callers (many destructive API commands are allowed under MCP policy), though mitigated by explicit policies, token isolation, and disabled local project/config discovery; regressions in policy exhaustiveness or context routing could leak tokens or host filesystem behavior.
Overview
Adds in-process CLI invocation for embedding hosts (e.g. MCP):
@sanity/cli/invokeSanityCliruns allowed oclif commands, returns{exitCode, output}, and never shells out.@sanity/cli-core/executionContextusesAsyncLocalStorageso each call can supply a per-invocation token (bypasses env/config/cache), stdout/stderr sinks, forced non-interactive behavior, and no filesystem project-root discovery (callers must pass flags like--project-id).SanityCommandlogging/warnings andgetCliTokenhonor this context; the normalsanitybinary is unchanged.MCP command policy is an exhaustive allow / deny / conditional table (
mcpPolicy) synced to the oclif manifest; denied commands look like unknown commands. Help is filtered to the invokable surface (plain text, no probing denied topics). Includes shell-styletokenizeCliArgs(no shell) and a small guard oncors addso programmatic runs do not stat the host cwd.Reviewed by Cursor Bugbot for commit ab297aa. Configure here.