diff --git a/rust/crates/sift_cli/CHANGELOG.md b/rust/crates/sift_cli/CHANGELOG.md index 65b5486b0..e2d95ef24 100644 --- a/rust/crates/sift_cli/CHANGELOG.md +++ b/rust/crates/sift_cli/CHANGELOG.md @@ -5,6 +5,16 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### What's New + +- Added an `--allow-create` MCP access tier alongside `--allow-destructive`. + The server now recognizes three tiers: read-only (default), create (enables + `create_*`, `upload_dataset`, `append_test_measurements`), and destructive + (implies create; enables `update_*`, `archive_*`, `unarchive_*`). The flag is + available on `sift-cli mcp`, `sift-cli agent install`, and + `sift-cli agent update`; blocked calls now name the exact remediation command + for the tier they need. + ## [v0.4.0] - August 5, 2026 ### What's New diff --git a/rust/crates/sift_cli/assets/skills/sift/SKILL.md b/rust/crates/sift_cli/assets/skills/sift/SKILL.md index fc73d0ede..f197e0fd2 100644 --- a/rust/crates/sift_cli/assets/skills/sift/SKILL.md +++ b/rust/crates/sift_cli/assets/skills/sift/SKILL.md @@ -79,6 +79,14 @@ exists. chart and numbers, do both and give the user both. - **Answer a question about how Sift works.** Call `search_docs`. Do not answer from memory, and cite the page you used. +- **Create an asset.** There is no `create_asset` MCP tool because Sift creates + assets implicitly on ingest. The MCP path is `upload_dataset` with the target + asset name; the asset is registered as a side effect. `upload_dataset` is a + create tool, so it is gated by `--allow-create`. If the server is read-only, + DO NOT reach for the REST `CreateAsset` RPC, `sift-cli import`, `sift_stream`, + or a gRPC client to make the asset another way. Surface the block: name the + gated tool and the exact `sift-cli agent update --allow-create` command, and + wait for the user to widen access. - **Evaluate rules against a run.** Find rules with `list_rules` and author rules with `create_rule`. To reuse the same rule set across many runs, bundle standard rules (`is_external: false`) into a template with `create_report_template`, @@ -98,11 +106,27 @@ exists. URL that a tool did not return. - **Confirm every write before you run it.** Show the user the proposed change and its target, then wait for approval. -- **Destructive tools are off by default.** `update_*`, `archive_*`, and - `unarchive_*` need `--allow-destructive`. If one is blocked, tell the user - that this access is disabled by default and ask for explicit approval. Never - enable it silently. The procedure is in - [references/agent-setup.md](references/agent-setup.md). +- **Write tools are off by default.** Read-only is the default access mode. + `create_*`, `upload_dataset`, and `append_test_measurements` need + `--allow-create`. `update_*`, `archive_*`, and `unarchive_*` need + `--allow-destructive` (which implies create). If a call is blocked, tell the + user that this access is disabled by default and ask for explicit approval. + Never widen access silently. +- **A blocked MCP tool is a user policy signal, not a transport error.** If + the MCP gate blocks a write, do NOT route around it — do not shell to + `sift-cli import`, `curl` against the REST API, `sift_client` Python, or + another MCP server that happens to be in destructive mode. Surface the + block and the exact remediation command; wait for the user to widen access. + The procedure is in [references/agent-setup.md](references/agent-setup.md). +- **"No dedicated MCP tool" is not the same as "can't do this in MCP".** + Several creates happen as side effects of other MCP tools — an asset is + created when `upload_dataset` names one that doesn't exist, a run is + created when a `create_report`/`create_test_report`/`upload_dataset` names + one. When the user asks for a create with no matching `create_*` tool, + look for the tool that creates it as a side effect before falling out of + MCP. If that side-effect tool is gated and blocked, that IS the block — + surface it, do not treat "no `create_asset` tool" as license to shell out + to REST/gRPC/`sift-cli import`. - **Choose one profile for the session and keep it.** Never switch profiles to recover from a failure. Surface the failure and ask the user. diff --git a/rust/crates/sift_cli/assets/skills/sift/references/agent-setup.md b/rust/crates/sift_cli/assets/skills/sift/references/agent-setup.md index f9e3b8f08..acc497425 100644 --- a/rust/crates/sift_cli/assets/skills/sift/references/agent-setup.md +++ b/rust/crates/sift_cli/assets/skills/sift/references/agent-setup.md @@ -23,29 +23,62 @@ skill: pass it as `sift-cli agent install --profile `. Run the command when the user asks you to install or approves the change. - Run `sift-cli agent update` to refresh every detected client together. It - preserves the existing profile and read-only or destructive access mode. - Switch every client to another named profile with - `sift-cli agent update --profile `, or return them to the default with - `sift-cli agent update --default-profile`. + preserves the existing profile and access mode. Switch every client to another + named profile with `sift-cli agent update --profile `, or return them to + the default with `sift-cli agent update --default-profile`. - If the CLI is outdated, relay the exact curl or PowerShell installer printed by `agent doctor` or `agent update`. After the user updates `sift-cli`, rerun `sift-cli agent update`. - Never repair or update only one detected client. If doctor reports mixed - access modes, ask the user to choose `sift-cli agent update --read-only` or - `sift-cli agent update --allow-destructive`. If it reports mixed profiles, + access modes, ask the user to choose `sift-cli agent update --read-only`, + `--allow-create`, or `--allow-destructive`. If it reports mixed profiles, ask for the intended profile and use `sift-cli agent update --profile ` or `sift-cli agent update --default-profile`. -## Enabling destructive tools +## Access modes -Destructive tools (`update_*`, `archive_*`, `unarchive_*`) are gated on -`--allow-destructive` and disabled by default. When a call is blocked: +Three tiers, each a superset of the last. All are opt-in per detected client +via `sift-cli agent update`: -1. Explain to the user that this access is disabled by default. +- **Read-only** (default) — no writes. Any create or destructive tool call + returns a structured error naming the remediation command. +- **Create** — enables `create_*`, `upload_dataset`, and `append_test_measurements` + (additive writes that don't modify existing data). Enable with + `sift-cli agent update --allow-create`. +- **Destructive** — enables everything, including `update_*`, `archive_*`, and + `unarchive_*`. Enable with `sift-cli agent update --allow-destructive`. + Implies create. + +When a call is blocked: + +1. Explain to the user which tier is required and that it's disabled by default. 2. Ask for explicit approval to enable it across every detected client. -3. Only after approval, run `sift-cli agent update --allow-destructive`. +3. Only after approval, run the exact command in the tool's error message + (`sift-cli agent update --allow-create` or `--allow-destructive`). 4. Ask the user to reload or restart the MCP client. 5. Wait for the user to confirm the restart before you retry the call. -Never enable destructive access silently. Restore safe mode with +Never widen access silently. Restore safe mode with `sift-cli agent update --read-only`. + +## Access modes are the ceiling for the whole session + +The MCP access tier is a policy signal for what the user has authorized, not +just an MCP-transport gate. If the tier blocks a write, DO NOT accomplish the +same write another way: + +- Do not shell to `sift-cli import`, `curl` against the Sift REST API, or + `sift_client` Python to route around a blocked MCP tool. +- Do not use one MCP server to make a write another MCP server would block + (e.g. `sift-dev` in destructive mode is not a bypass for `sift` in read-only). +- Do not compose reads and side-effects to imitate a write (creating an + annotation with a "create" verb is a create, whichever path runs it). + +Treat a blocked MCP tool the same way you would treat any other explicit user +denial: surface the block, name the exact remediation command from the error +payload, and wait for the user to widen access before retrying. The whole +point of read-only and create tiers is to let a user register the agent with +narrow authority; a fallback path that ignores the tier is a foot-gun that +defeats the setting. + +Reads work everywhere at every tier. This rule is only about writes. diff --git a/rust/crates/sift_cli/assets/skills/sift/references/cli.md b/rust/crates/sift_cli/assets/skills/sift/references/cli.md index 7517e6669..a1750d877 100644 --- a/rust/crates/sift_cli/assets/skills/sift/references/cli.md +++ b/rust/crates/sift_cli/assets/skills/sift/references/cli.md @@ -65,6 +65,12 @@ per session. The rest apply to each subcommand invocation. (imports, config changes), surface the final proposed command and the target (asset, run, profile) to the user and wait for approval before running. + **Never use the CLI to route around a blocked MCP write.** If an MCP + tool returned a "disabled by default" error naming an + `sift-cli agent update --allow-...` command, honor that block. Do not + shell to `sift-cli import`, `curl`, or a Python client to accomplish the + same operation. Surface the block per [references/agent-setup.md](agent-setup.md) + and wait for the user to widen access first. 5. **Use absolute paths.** Pass absolute paths for any file argument so the command does not depend on the shell's current directory. 6. **For imports, pass `--wait`.** With `--wait` the CLI blocks until the diff --git a/rust/crates/sift_cli/src/cli/mod.rs b/rust/crates/sift_cli/src/cli/mod.rs index 29e6332de..07d211115 100644 --- a/rust/crates/sift_cli/src/cli/mod.rs +++ b/rust/crates/sift_cli/src/cli/mod.rs @@ -74,9 +74,15 @@ pub enum Cmd { #[derive(clap::Args)] pub struct McpArgs { + /// Expose create tools (creates, ingest, appends). When omitted, create + /// tool calls return an error instructing the caller to relaunch the + /// server with this flag. Implied by `--allow-destructive`. + #[arg(long)] + pub allow_create: bool, + /// Expose destructive tools (updates, archives, restores). When omitted, /// destructive tool calls return an error instructing the caller to - /// relaunch the server with this flag. + /// relaunch the server with this flag. Also enables create tools. #[arg(long)] pub allow_destructive: bool, } @@ -115,19 +121,29 @@ pub enum AgentCmd { #[derive(clap::Args)] pub struct AgentInstallArgs { - /// Enable tools that modify or archive resources for every detected MCP client + /// Enable tools that create new resources for every detected MCP client + #[arg(long)] + pub allow_create: bool, + + /// Enable tools that modify or archive resources for every detected MCP client. + /// Also enables create tools. #[arg(long)] pub allow_destructive: bool, } #[derive(clap::Args)] pub struct AgentUpdateArgs { - /// Enable tools that modify or archive resources for every detected MCP client + /// Enable tools that create new resources for every detected MCP client + #[arg(long, conflicts_with = "read_only")] + pub allow_create: bool, + + /// Enable tools that modify or archive resources for every detected MCP client. + /// Also enables create tools. #[arg(long, conflicts_with = "read_only")] pub allow_destructive: bool, - /// Disable destructive tools for every detected MCP client - #[arg(long, conflicts_with = "allow_destructive")] + /// Disable create and destructive tools for every detected MCP client + #[arg(long, conflicts_with_all = ["allow_create", "allow_destructive"])] pub read_only: bool, /// Switch every detected MCP client back to the default profile diff --git a/rust/crates/sift_cli/src/cmd/agent/config.rs b/rust/crates/sift_cli/src/cmd/agent/config.rs index 6822a15d4..f60a3c849 100644 --- a/rust/crates/sift_cli/src/cmd/agent/config.rs +++ b/rust/crates/sift_cli/src/cmd/agent/config.rs @@ -418,8 +418,10 @@ fn mcp_args(registration: &Registration) -> Vec { args.push("--profile".to_string()); args.push(profile.clone()); } - if registration.access == AccessMode::Destructive { - args.push("--allow-destructive".to_string()); + match registration.access { + AccessMode::ReadOnly => {} + AccessMode::Create => args.push("--allow-create".to_string()), + AccessMode::Destructive => args.push("--allow-destructive".to_string()), } args } @@ -582,8 +584,8 @@ fn classify_command(command: &str, args: &[String], environment: &Environment) - fn registration_from_args(args: &[String]) -> Option { match args { [mcp] if mcp == "mcp" => Some(Registration::new(AccessMode::ReadOnly, Profile::Default)), - [mcp, destructive] if mcp == "mcp" && destructive == "--allow-destructive" => { - Some(Registration::new(AccessMode::Destructive, Profile::Default)) + [mcp, access] if mcp == "mcp" => { + access_from_flag(access).map(|a| Registration::new(a, Profile::Default)) } [mcp, profile_flag, profile] if mcp == "mcp" && profile_flag == "--profile" && valid_profile(profile) => @@ -593,21 +595,23 @@ fn registration_from_args(args: &[String]) -> Option { Profile::Named(profile.clone()), )) } - [mcp, profile_flag, profile, destructive] - if mcp == "mcp" - && profile_flag == "--profile" - && valid_profile(profile) - && destructive == "--allow-destructive" => + [mcp, profile_flag, profile, access] + if mcp == "mcp" && profile_flag == "--profile" && valid_profile(profile) => { - Some(Registration::new( - AccessMode::Destructive, - Profile::Named(profile.clone()), - )) + access_from_flag(access).map(|a| Registration::new(a, Profile::Named(profile.clone()))) } _ => None, } } +fn access_from_flag(flag: &str) -> Option { + match flag { + "--allow-create" => Some(AccessMode::Create), + "--allow-destructive" => Some(AccessMode::Destructive), + _ => None, + } +} + fn valid_profile(profile: &str) -> bool { !profile.is_empty() && !profile.starts_with('-') } @@ -940,8 +944,13 @@ mod tests { #[test] fn only_exact_sift_mcp_argument_shapes_are_managed() { assert!(registration_from_args(&args(&["mcp"])).is_some()); + assert!(registration_from_args(&args(&["mcp", "--allow-create"])).is_some()); assert!(registration_from_args(&args(&["mcp", "--allow-destructive"])).is_some()); assert!(registration_from_args(&args(&["mcp", "--profile", "localdev"])).is_some()); + assert!( + registration_from_args(&args(&["mcp", "--profile", "localdev", "--allow-create",])) + .is_some() + ); assert!( registration_from_args(&args(&[ "mcp", @@ -957,6 +966,7 @@ mod tests { args(&["mcp", "--profile"]), args(&["mcp", "--profile", "--allow-destructive"]), args(&["mcp", "--allow-destructive", "--profile", "localdev"]), + args(&["mcp", "--allow-create", "--profile", "localdev"]), args(&["mcp", "--profile", "localdev", "--extra"]), ] { assert_eq!(registration_from_args(&custom), None); diff --git a/rust/crates/sift_cli/src/cmd/agent/mod.rs b/rust/crates/sift_cli/src/cmd/agent/mod.rs index 38819a500..6d1d3c590 100644 --- a/rust/crates/sift_cli/src/cmd/agent/mod.rs +++ b/rust/crates/sift_cli/src/cmd/agent/mod.rs @@ -37,6 +37,7 @@ fn warning_status() -> StyledContent<&'static str> { #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub(super) enum AccessMode { ReadOnly, + Create, Destructive, } @@ -44,6 +45,7 @@ impl AccessMode { fn label(self) -> &'static str { match self { Self::ReadOnly => "read-only", + Self::Create => "create tools enabled", Self::Destructive => "destructive tools enabled", } } @@ -201,6 +203,8 @@ impl Environment { pub fn install(profile: Option, args: AgentInstallArgs) -> Result { let access = if args.allow_destructive { AccessMode::Destructive + } else if args.allow_create { + AccessMode::Create } else { AccessMode::ReadOnly }; @@ -231,6 +235,8 @@ pub async fn update(profile: Option, args: AgentUpdateArgs) -> Result, args: AgentUpdateArgs) -> Result` or `--default-profile`."); diff --git a/rust/crates/sift_cli/src/cmd/agent/tests.rs b/rust/crates/sift_cli/src/cmd/agent/tests.rs index 9d2145f86..e765346c6 100644 --- a/rust/crates/sift_cli/src/cmd/agent/tests.rs +++ b/rust/crates/sift_cli/src/cmd/agent/tests.rs @@ -466,6 +466,16 @@ fn update_access_flags_are_mutually_exclusive() { ]) .is_err() ); + assert!( + crate::cli::Args::try_parse_from([ + "sift-cli", + "agent", + "update", + "--allow-create", + "--read-only", + ]) + .is_err() + ); } #[test] diff --git a/rust/crates/sift_cli/src/cmd/mcp.rs b/rust/crates/sift_cli/src/cmd/mcp.rs index 623f35e1b..0f089d6fe 100644 --- a/rust/crates/sift_cli/src/cmd/mcp.rs +++ b/rust/crates/sift_cli/src/cmd/mcp.rs @@ -46,6 +46,7 @@ pub async fn run(ctx: Context, args: McpArgs, app_uri: String) -> Result. -5. Every tool with `destructive_hint = true` MUST call `self.require_destructive()?` as the - first line of its handler. The server is launched without destructive tools by default; the - gate returns an `INVALID_REQUEST` telling the caller to relaunch with `--allow-destructive`. - Skipping this call ships a destructive tool that is always on and bypasses the flag. +5. Every write handler MUST gate itself as the FIRST line of the handler. Additive-write tools + (`destructive_hint = false`) call `self.require_create()?`; destructive tools + (`destructive_hint = true`) call `self.require_destructive()?`. The server is launched + read-only by default; each gate returns an `INVALID_REQUEST` naming the exact + `sift-cli agent update --allow-...` command the caller should run. Skipping the gate ships a + write tool that is always on and bypasses the flags. Note: destructive implies create, so + destructive tools do not additionally need to call `require_create`. 6. Do not mirror the API one-to-one. Run Step 0 before writing anything. --- @@ -251,11 +254,13 @@ pub async fn list_webhooks(&self, params: Parameters) -> error::McpR - Validate before calling the service. Return `ErrorData::invalid_params(...)` or `ErrorData::resource_not_found(...)` for bad input. `get_data` shows multi-field validation. -- **Gate destructive handlers.** If the tool sets `destructive_hint = true`, call - `self.require_destructive()?` as the FIRST line of the handler — before parameter - destructuring, validation, or any service call. The gate returns an `INVALID_REQUEST` when - the server was launched without `--allow-destructive`, so no gRPC traffic and no partial - work occurs when the flag is off. `tool/assets/mod.rs::update_asset` is the reference. +- **Gate write handlers.** Additive-write tools (`destructive_hint = false`) call + `self.require_create()?` as the FIRST line of the handler; destructive tools + (`destructive_hint = true`) call `self.require_destructive()?` as the FIRST line. + Both gates run before parameter destructuring, validation, or any service call, and return + an `INVALID_REQUEST` naming the exact remediation command when the server was launched + without the corresponding flag. `tool/assets/mod.rs::update_asset` is the destructive + reference; `tool/annotations/mod.rs::create_annotation` is the create reference. - Always return `CallToolResult::structured(json!({ ... }))`. - Annotate: `annotations(title = "/", read_only_hint = , destructive_hint = , idempotent_hint = )`. Read tools set `read_only_hint = true` and may omit the other two. Write tools set @@ -674,9 +679,10 @@ Run through this before declaring the tool done: adapted to that resource's own text fields and phrased as a direct instruction. - [ ] `read_only_hint` is correct, and write tools set `destructive_hint` / `idempotent_hint` per the Step 4 mapping. Write tools confirm the destination via `next_step`. -- [ ] Every handler with `destructive_hint = true` calls `self.require_destructive()?` as its - first line, and has a test that asserts the gate returns `INVALID_REQUEST` when the - server is constructed with `allow_destructive = false`. See +- [ ] Every write handler gates itself as the first line: `self.require_create()?` for + additive writes (`destructive_hint = false`), `self.require_destructive()?` for + destructive writes. Tests assert each gate returns `INVALID_REQUEST` when the server is + constructed with the corresponding flag off. See `tool/assets/test.rs::update_asset_blocked_without_allow_destructive`. - [ ] Service registered in `server/mod.rs` and the router merged. - [ ] Service tests added, covering single page, pagination, `limit`, and an error path. A mock diff --git a/rust/crates/sift_mcp/src/lib.rs b/rust/crates/sift_mcp/src/lib.rs index 08c093dd5..29e8a896c 100644 --- a/rust/crates/sift_mcp/src/lib.rs +++ b/rust/crates/sift_mcp/src/lib.rs @@ -19,6 +19,7 @@ pub async fn run( credentials: Credentials, use_tls: bool, app_uri: String, + allow_create: bool, allow_destructive: bool, ) -> Result<()> { let channel = SiftChannelBuilder::new(credentials) @@ -27,7 +28,7 @@ pub async fn run( .build() .context("failed to build gRPC channel to connect to Sift")?; - let service = SiftMcpServer::new(channel, app_uri, allow_destructive) + let service = SiftMcpServer::new(channel, app_uri, allow_create, allow_destructive) .serve(stdio()) .await .context("failed to start MCP server")?; diff --git a/rust/crates/sift_mcp/src/server/mod.rs b/rust/crates/sift_mcp/src/server/mod.rs index e4be60508..364735373 100644 --- a/rust/crates/sift_mcp/src/server/mod.rs +++ b/rust/crates/sift_mcp/src/server/mod.rs @@ -40,6 +40,7 @@ pub struct SiftMcpServer { pub docs_service: DocsService, pub user_service: UserService, + pub allow_create: bool, pub allow_destructive: bool, } @@ -83,7 +84,12 @@ impl ServerHandler for SiftMcpServer { } impl SiftMcpServer { - pub fn new(channel: SiftChannel, app_uri: String, allow_destructive: bool) -> Self { + pub fn new( + channel: SiftChannel, + app_uri: String, + allow_create: bool, + allow_destructive: bool, + ) -> Self { // Add more routers here as new tool groups are introduced, e.g. // tool_router.merge(Self::ingestion_router()) let mut tool_router = Self::assets_router(); @@ -140,23 +146,44 @@ impl SiftMcpServer { user_service, tool_router, prompt_router, + allow_create, allow_destructive, } } - /// Gate for destructive tool handlers. Returns an error the calling agent - /// can relay to the user when the server was launched without - /// `--allow-destructive`. - pub(crate) fn require_destructive(&self) -> Result<(), ErrorData> { - if self.allow_destructive { + pub(crate) fn require_create(&self) -> Result<(), ErrorData> { + if self.allow_create || self.allow_destructive { return Ok(()); } Err(ErrorData::invalid_request( - "This tool is destructive and is disabled. Ask the user for explicit \ - approval to enable destructive Sift tools. If they approve, run \ - `sift-cli agent update --allow-destructive` to update every detected \ + "This is a create tool. The MCP server was launched without \ + `--allow-create`, so tools that create new resources are disabled. \ + Ask the user for explicit approval to enable them. If they approve, \ + run `sift-cli agent update --allow-create` to update every detected \ client together, then ask the user to reload or restart their MCP \ client. Do not retry until they confirm the client has restarted.", + Some(serde_json::json!({ + "status": "stopped", + "reason": "CreateToolsDisabled", + "requires_user_approval": true, + "remediation_command": "sift-cli agent update --allow-create", + "restart_required": true, + })), + )) + } + + pub(crate) fn require_destructive(&self) -> Result<(), ErrorData> { + if self.allow_destructive { + return Ok(()); + } + Err(ErrorData::invalid_request( + "This is a destructive tool. The MCP server was launched without \ + `--allow-destructive`, so tools that modify or archive resources \ + are disabled. Ask the user for explicit approval to enable them. \ + If they approve, run `sift-cli agent update --allow-destructive` \ + to update every detected client together, then ask the user to \ + reload or restart their MCP client. Do not retry until they \ + confirm the client has restarted.", Some(serde_json::json!({ "status": "stopped", "reason": "DestructiveToolsDisabled", diff --git a/rust/crates/sift_mcp/src/tool/annotations/mod.rs b/rust/crates/sift_mcp/src/tool/annotations/mod.rs index ce90546f1..54fd19716 100644 --- a/rust/crates/sift_mcp/src/tool/annotations/mod.rs +++ b/rust/crates/sift_mcp/src/tool/annotations/mod.rs @@ -202,6 +202,8 @@ impl SiftMcpServer { &self, params: Parameters, ) -> error::McpResult { + self.require_create()?; + let Parameters(CreateAnnotationParams { name, description, diff --git a/rust/crates/sift_mcp/src/tool/annotations/test.rs b/rust/crates/sift_mcp/src/tool/annotations/test.rs index 4f18363e9..82ccd52c6 100644 --- a/rust/crates/sift_mcp/src/tool/annotations/test.rs +++ b/rust/crates/sift_mcp/src/tool/annotations/test.rs @@ -23,7 +23,7 @@ async fn server_with_mock(mock: MockAnnotationServiceImpl) -> (SiftMcpServer, Jo }); ( - SiftMcpServer::new(channel, String::from("https://app.test.local"), true), + SiftMcpServer::new(channel, String::from("https://app.test.local"), true, true), handle, ) } diff --git a/rust/crates/sift_mcp/src/tool/assets/test.rs b/rust/crates/sift_mcp/src/tool/assets/test.rs index 63fd162ba..b96fe88c0 100644 --- a/rust/crates/sift_mcp/src/tool/assets/test.rs +++ b/rust/crates/sift_mcp/src/tool/assets/test.rs @@ -36,6 +36,7 @@ async fn server_with_mock_and_flag( SiftMcpServer::new( channel, String::from("https://app.test.local"), + false, allow_destructive, ), handle, diff --git a/rust/crates/sift_mcp/src/tool/channels/test.rs b/rust/crates/sift_mcp/src/tool/channels/test.rs index 003f1b1bc..c4cb22d4a 100644 --- a/rust/crates/sift_mcp/src/tool/channels/test.rs +++ b/rust/crates/sift_mcp/src/tool/channels/test.rs @@ -25,7 +25,7 @@ async fn server_with_mock(mock: MockChannelServiceImpl) -> (SiftMcpServer, JoinH }); ( - SiftMcpServer::new(channel, String::from("https://app.test.local"), true), + SiftMcpServer::new(channel, String::from("https://app.test.local"), true, true), handle, ) } diff --git a/rust/crates/sift_mcp/src/tool/data/mod.rs b/rust/crates/sift_mcp/src/tool/data/mod.rs index a39eaf606..ab2c34863 100644 --- a/rust/crates/sift_mcp/src/tool/data/mod.rs +++ b/rust/crates/sift_mcp/src/tool/data/mod.rs @@ -420,6 +420,8 @@ impl SiftMcpServer { &self, params: Parameters, ) -> error::McpResult { + self.require_create()?; + let Parameters(UploadDatasetParams { asset, run_name, diff --git a/rust/crates/sift_mcp/src/tool/data/test.rs b/rust/crates/sift_mcp/src/tool/data/test.rs index 9b605c8f7..648327f01 100644 --- a/rust/crates/sift_mcp/src/tool/data/test.rs +++ b/rust/crates/sift_mcp/src/tool/data/test.rs @@ -30,7 +30,12 @@ async fn server_with_mocks( }); ( - SiftMcpServer::new(channel, String::from("https://api.test.local"), false), + SiftMcpServer::new( + channel, + String::from("https://api.test.local"), + false, + false, + ), handle, ) } diff --git a/rust/crates/sift_mcp/src/tool/docs/test.rs b/rust/crates/sift_mcp/src/tool/docs/test.rs index 2f9bd4dc4..b5d695d11 100644 --- a/rust/crates/sift_mcp/src/tool/docs/test.rs +++ b/rust/crates/sift_mcp/src/tool/docs/test.rs @@ -24,7 +24,7 @@ async fn server_with_mock(mock: MockDocsServiceImpl) -> (SiftMcpServer, JoinHand .unwrap(); }); - let server = SiftMcpServer::new(channel, "https://app.test.local".into(), true); + let server = SiftMcpServer::new(channel, "https://app.test.local".into(), true, true); (server, handle) } diff --git a/rust/crates/sift_mcp/src/tool/explore/test.rs b/rust/crates/sift_mcp/src/tool/explore/test.rs index 290f1fc47..f69fd3c7b 100644 --- a/rust/crates/sift_mcp/src/tool/explore/test.rs +++ b/rust/crates/sift_mcp/src/tool/explore/test.rs @@ -10,7 +10,7 @@ const APP_URI: &str = "https://app.siftstack.com"; async fn server_for_explore(app_uri: &str) -> SiftMcpServer { let (client, _server) = tokio::io::duplex(1024); let channel = memory_sift_channel(client).await; - SiftMcpServer::new(channel, app_uri.to_string(), true) + SiftMcpServer::new(channel, app_uri.to_string(), true, true) } fn structured_field(result: rmcp::model::CallToolResult, key: &str) -> Value { diff --git a/rust/crates/sift_mcp/src/tool/report_templates/mod.rs b/rust/crates/sift_mcp/src/tool/report_templates/mod.rs index 4c885fe0a..3eacf0f95 100644 --- a/rust/crates/sift_mcp/src/tool/report_templates/mod.rs +++ b/rust/crates/sift_mcp/src/tool/report_templates/mod.rs @@ -168,6 +168,8 @@ impl SiftMcpServer { &self, params: Parameters, ) -> error::McpResult { + self.require_create()?; + let Parameters(CreateReportTemplateParams { name, description, diff --git a/rust/crates/sift_mcp/src/tool/report_templates/test.rs b/rust/crates/sift_mcp/src/tool/report_templates/test.rs index c1ec33178..97719ee84 100644 --- a/rust/crates/sift_mcp/src/tool/report_templates/test.rs +++ b/rust/crates/sift_mcp/src/tool/report_templates/test.rs @@ -13,11 +13,12 @@ use super::{CreateReportTemplateParams, ReportTemplateListParams, UpdateReportTe use crate::{server::SiftMcpServer, tool::common::test_support::structured_field}; async fn server_with_mock(mock: MockReportTemplateServiceImpl) -> (SiftMcpServer, JoinHandle<()>) { - server_with_mock_and_flag(mock, true).await + server_with_mock_and_flags(mock, true, true).await } -async fn server_with_mock_and_flag( +async fn server_with_mock_and_flags( mock: MockReportTemplateServiceImpl, + allow_create: bool, allow_destructive: bool, ) -> (SiftMcpServer, JoinHandle<()>) { let (client, server) = tokio::io::duplex(1024); @@ -35,6 +36,7 @@ async fn server_with_mock_and_flag( SiftMcpServer::new( channel, String::from("https://app.test.local"), + allow_create, allow_destructive, ), handle, @@ -293,10 +295,24 @@ async fn update_report_template_rejects_both_rule_shapes() { assert_eq!(err.code, ErrorCode::INVALID_PARAMS); } +#[tokio::test] +async fn create_report_template_blocked_without_allow_create() { + let mock = MockReportTemplateServiceImpl::new(); + let (server, _h) = server_with_mock_and_flags(mock, false, false).await; + + let err = server + .create_report_template(Parameters(create_params())) + .await + .expect_err("expected create gate to reject the call"); + + assert_eq!(err.code, ErrorCode::INVALID_REQUEST); + assert!(err.message.contains("--allow-create")); +} + #[tokio::test] async fn update_report_template_blocked_without_allow_destructive() { let mock = MockReportTemplateServiceImpl::new(); - let (server, _h) = server_with_mock_and_flag(mock, false).await; + let (server, _h) = server_with_mock_and_flags(mock, false, false).await; let mut params = update_params(); params.name = Some("renamed".into()); diff --git a/rust/crates/sift_mcp/src/tool/reports/mod.rs b/rust/crates/sift_mcp/src/tool/reports/mod.rs index bc811ff72..7f5aa1098 100644 --- a/rust/crates/sift_mcp/src/tool/reports/mod.rs +++ b/rust/crates/sift_mcp/src/tool/reports/mod.rs @@ -236,6 +236,8 @@ impl SiftMcpServer { ) )] pub async fn create_report(&self, params: Parameters) -> error::McpResult { + self.require_create()?; + let Parameters(CreateReportParams { run_id, name, diff --git a/rust/crates/sift_mcp/src/tool/reports/test.rs b/rust/crates/sift_mcp/src/tool/reports/test.rs index b5d153e6d..5b48c7747 100644 --- a/rust/crates/sift_mcp/src/tool/reports/test.rs +++ b/rust/crates/sift_mcp/src/tool/reports/test.rs @@ -49,7 +49,7 @@ async fn server_with_mock(mock: MockReportServiceImpl) -> (SiftMcpServer, JoinHa }); ( - SiftMcpServer::new(channel, String::from("https://app.test.local"), true), + SiftMcpServer::new(channel, String::from("https://app.test.local"), true, true), handle, ) } @@ -71,7 +71,7 @@ async fn server_with_dual_mocks( }); ( - SiftMcpServer::new(channel, String::from("https://app.test.local"), true), + SiftMcpServer::new(channel, String::from("https://app.test.local"), true, true), handle, ) } diff --git a/rust/crates/sift_mcp/src/tool/rules/mod.rs b/rust/crates/sift_mcp/src/tool/rules/mod.rs index 5835684b7..16db80a6c 100644 --- a/rust/crates/sift_mcp/src/tool/rules/mod.rs +++ b/rust/crates/sift_mcp/src/tool/rules/mod.rs @@ -226,6 +226,8 @@ impl SiftMcpServer { ) )] pub async fn create_rule(&self, params: Parameters) -> error::McpResult { + self.require_create()?; + let Parameters(RuleDefinitionParams { rule_json }) = params; let update = parse_rule_definition(&rule_json)?; diff --git a/rust/crates/sift_mcp/src/tool/runs/test.rs b/rust/crates/sift_mcp/src/tool/runs/test.rs index 4ef542324..9cc60df5e 100644 --- a/rust/crates/sift_mcp/src/tool/runs/test.rs +++ b/rust/crates/sift_mcp/src/tool/runs/test.rs @@ -39,7 +39,7 @@ async fn server_with_mock(mock: MockRunServiceImpl) -> (SiftMcpServer, JoinHandl }); ( - SiftMcpServer::new(channel, String::from("https://app.test.local"), true), + SiftMcpServer::new(channel, String::from("https://app.test.local"), true, true), handle, ) } diff --git a/rust/crates/sift_mcp/src/tool/test_reports/mod.rs b/rust/crates/sift_mcp/src/tool/test_reports/mod.rs index 7b9cd1ca3..6a1fb3d78 100644 --- a/rust/crates/sift_mcp/src/tool/test_reports/mod.rs +++ b/rust/crates/sift_mcp/src/tool/test_reports/mod.rs @@ -362,6 +362,8 @@ impl SiftMcpServer { &self, params: Parameters, ) -> error::McpResult { + self.require_create()?; + let Parameters(CreateTestReportParams { report_json }) = params; let report_spec = parse_report_spec(&report_json)?; @@ -439,6 +441,8 @@ impl SiftMcpServer { &self, params: Parameters, ) -> error::McpResult { + self.require_create()?; + let Parameters(AppendMeasurementsParams { test_report_id, test_step_id, diff --git a/rust/crates/sift_mcp/src/tool/test_reports/test.rs b/rust/crates/sift_mcp/src/tool/test_reports/test.rs index 4a92abdd2..1097c8348 100644 --- a/rust/crates/sift_mcp/src/tool/test_reports/test.rs +++ b/rust/crates/sift_mcp/src/tool/test_reports/test.rs @@ -25,7 +25,7 @@ async fn server_with_mock(mock: MockTestReportServiceImpl) -> (SiftMcpServer, Jo }); ( - SiftMcpServer::new(channel, String::from("https://app.test.local"), true), + SiftMcpServer::new(channel, String::from("https://app.test.local"), true, true), handle, ) } diff --git a/rust/crates/sift_mcp/src/tool/users/test.rs b/rust/crates/sift_mcp/src/tool/users/test.rs index 322186a92..44054399f 100644 --- a/rust/crates/sift_mcp/src/tool/users/test.rs +++ b/rust/crates/sift_mcp/src/tool/users/test.rs @@ -64,7 +64,12 @@ async fn server_with_mocks( }); ( - SiftMcpServer::new(channel, String::from("https://app.test.local"), false), + SiftMcpServer::new( + channel, + String::from("https://app.test.local"), + false, + false, + ), handle, ) }