Skip to content

Commit e4a506e

Browse files
authored
fix(security): disable mcp action apply writes
Make the MCP server read-only by removing advertised action apply support, blocking cached action_apply calls before registry dispatch, and updating docs/tests to require out-of-band CLI/TUI/Raycast confirmation for writes.
1 parent a283397 commit e4a506e

13 files changed

Lines changed: 56 additions & 128 deletions

File tree

README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ The README uses a GIF so the preview renders directly on GitHub. The docs homepa
2929
| --- | --- | --- |
3030
| TUI | Dashboard, inventory, findings, analysis, fix plan, backup preview, action queue | Read-only until a confirmed action is applied |
3131
| CLI | Scriptable scan, doctor, policy, SARIF, snapshot, schedule, backup, and action commands | Read-only unless explicit output/export paths or `--confirm` actions are requested |
32-
| MCP server | Stdio tools/resources/prompts for AI clients | Direct apply only through shared action-registry IDs with disclosure, confirmation, redaction, and audit logging |
32+
| MCP server | Stdio tools/resources/prompts for AI clients | Read-only; can list/preview actions, but writes must be applied in CLI/TUI/Raycast |
3333
| Raycast | macOS companion commands plus confirmed Nightward Actions | Clipboard/report-folder actions plus confirmation-gated writes |
3434
| GitHub Action | Workspace policy and SARIF checks | Writes only requested CI outputs |
3535
| Trunk plugin | Local workspace policy/analyze linters | Emits SARIF to stdout |
@@ -296,7 +296,7 @@ Secret values are never emitted in scan JSON, findings output, fix-plan JSON, Ma
296296

297297
`nw analyze` turns scan findings and classifications into explainable signals. It does not claim a package, server, binary, or URL is safe. It reports what Nightward can prove from local structure, why it matters, and how confident the signal is.
298298

299-
Default analysis is offline and built in. Optional providers are discovered by `providers doctor`; Nightward does not call online services unless a user explicitly selects providers and opts into network-capable behavior. The CLI/TUI/Raycast/MCP action layer can install known provider CLIs after confirmation. Explicit local providers are `gitleaks`, `trufflehog`, `semgrep`, and `syft`. Online-capable providers are `trivy`, `osv-scanner`, `grype`, `scorecard`, and `socket`, and they require explicit online-provider opt-in. Socket support creates a remote Socket scan artifact from dependency manifest metadata; Nightward does not fetch or normalize remote Socket reports in v1.
299+
Default analysis is offline and built in. Optional providers are discovered by `providers doctor`; Nightward does not call online services unless a user explicitly selects providers and opts into network-capable behavior. The CLI/TUI/Raycast action layer can install known provider CLIs after confirmation. MCP can list and preview those actions, but cannot apply local writes. Explicit local providers are `gitleaks`, `trufflehog`, `semgrep`, and `syft`. Online-capable providers are `trivy`, `osv-scanner`, `grype`, `scorecard`, and `socket`, and they require explicit online-provider opt-in. Socket support creates a remote Socket scan artifact from dependency manifest metadata; Nightward does not fetch or normalize remote Socket reports in v1.
300300

301301
Provider runs use explicit skip/block/ready states, timeouts, bounded output capture, and redacted metadata only. Oversized provider stdout fails closed as a provider warning instead of being partially parsed. Semgrep execution requires a repo-local config file so Nightward does not use automatic rule discovery by default.
302302

@@ -356,7 +356,7 @@ Nightward can expose local context and bounded Nightward action workflows to MCP
356356
}
357357
```
358358

359-
The server supports scan, doctor, findings, finding/signal explanation, analysis, fix-plan, policy-check, report history/diff, action list/preview/apply, rules, providers, resources, and prompts. It uses stdio only, does not open a network listener, and cannot rewrite arbitrary MCP or agent config. `nightward_action_apply` can apply only shared action-registry IDs after disclosure acceptance, `confirm: true`, action availability checks, redacted output, and audit logging.
359+
The server supports scan, doctor, findings, finding/signal explanation, analysis, fix-plan, policy-check, report history/diff, action list/preview, rules, providers, resources, and prompts. It uses stdio only, does not open a network listener, and cannot rewrite arbitrary MCP or agent config. MCP clients cannot apply local writes because tool-call arguments are not an out-of-band local confirmation channel; use the CLI, TUI, or Raycast extension to apply previewed actions.
360360

361361
## GitHub Action
362362

crates/nightward-core/src/mcpserver.rs

Lines changed: 29 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::actions::{self, ApplyOptions};
1+
use crate::actions;
22
use crate::analysis::{run as analyze, Options as AnalysisOptions};
33
use crate::fixplan::{plan as fix_plan, Selector};
44
use crate::inventory::{home_dir_from_env, load_report, redact_text, scan_home, scan_workspace};
@@ -97,7 +97,7 @@ fn initialize_result(requested: Option<&str>) -> Value {
9797
"version": env!("CARGO_PKG_VERSION"),
9898
"description": "Local-first AI agent, MCP, provider, and dotfiles security posture."
9999
},
100-
"instructions": "Nightward returns redacted local security posture. Write-capable MCP calls are limited to the shared Nightward action registry and require disclosure acceptance plus explicit confirmation."
100+
"instructions": "Nightward returns redacted local security posture. MCP is read-only: it can list and preview bounded actions, but local writes must be applied out-of-band in the Nightward CLI, TUI, or Raycast extension."
101101
})
102102
}
103103

@@ -187,13 +187,6 @@ fn tools() -> Vec<Value> {
187187
schema_action_id(),
188188
read_only_annotations("Action preview", false),
189189
),
190-
tool(
191-
"nightward_action_apply",
192-
"Action Apply",
193-
"Apply one bounded Nightward action after disclosure acceptance and confirm: true.",
194-
schema_action_apply(),
195-
write_annotations("Action apply", true, true),
196-
),
197190
tool(
198191
"nightward_rules",
199192
"Nightward Rules",
@@ -349,16 +342,6 @@ fn read_only_annotations(title: &str, open_world: bool) -> Value {
349342
})
350343
}
351344

352-
fn write_annotations(title: &str, destructive: bool, open_world: bool) -> Value {
353-
json!({
354-
"title": title,
355-
"readOnlyHint": false,
356-
"destructiveHint": destructive,
357-
"idempotentHint": false,
358-
"openWorldHint": open_world
359-
})
360-
}
361-
362345
fn read_resource(params: Value, home: &Path) -> Result<Value> {
363346
let uri = params
364347
.get("uri")
@@ -401,7 +384,7 @@ fn read_prompt(params: Value) -> Result<Value> {
401384
let finding_id = string_arg(&args, "finding_id");
402385
let text = match name {
403386
"audit_my_ai_setup" => {
404-
"Use Nightward MCP tools to run nightward_scan, nightward_analysis, and nightward_policy_check with compact output. Explain the highest-risk AI/MCP configuration issues, provider posture, and the safest next actions. Do not apply actions unless I explicitly ask and confirm them."
387+
"Use Nightward MCP tools to run nightward_scan, nightward_analysis, and nightward_policy_check with compact output. Explain the highest-risk AI/MCP configuration issues, provider posture, and the safest next actions. MCP is read-only, so preview any relevant action and tell me the CLI/TUI/Raycast path for applying it."
405388
}
406389
"explain_top_risks" => {
407390
"Use nightward_findings and nightward_analysis to identify the top risks. Explain what can actually break or leak, what is probably just review noise, and what should be fixed first."
@@ -410,7 +393,7 @@ fn read_prompt(params: Value) -> Result<Value> {
410393
return Ok(prompt_result(
411394
"Generate a safe Nightward fix workflow.",
412395
format!(
413-
"Use nightward_explain_finding and nightward_fix_plan for finding `{}`. If a bounded registry action is relevant, use nightward_action_preview first. Do not call nightward_action_apply unless I explicitly accept the disclosure and provide confirm: true.",
396+
"Use nightward_explain_finding and nightward_fix_plan for finding `{}`. If a bounded registry action is relevant, use nightward_action_preview first. MCP cannot apply local writes, so tell me how to apply the previewed action in the Nightward CLI, TUI, or Raycast extension.",
414397
if finding_id.is_empty() {
415398
"<finding-id>"
416399
} else {
@@ -420,7 +403,7 @@ fn read_prompt(params: Value) -> Result<Value> {
420403
));
421404
}
422405
"set_up_providers" => {
423-
"Use nightward_providers and nightward_actions_list to show missing, blocked, selected, and online-capable providers. Recommend provider.install/provider.enable actions only through nightward_action_preview, and call out online/network behavior before any apply."
406+
"Use nightward_providers and nightward_actions_list to show missing, blocked, selected, and online-capable providers. Recommend provider.install/provider.enable actions only through nightward_action_preview, call out online/network behavior, and tell me to apply writes in the Nightward CLI, TUI, or Raycast extension."
424407
}
425408
"compare_reports" => {
426409
"Use nightward_report_history and nightward_report_changes to compare the last two reports. Summarize new, removed, and changed findings, then recommend which changes actually matter."
@@ -456,6 +439,11 @@ fn call_tool_inner(params: Value, home: &Path) -> Result<Value> {
456439
.get("name")
457440
.and_then(Value::as_str)
458441
.ok_or_else(|| anyhow!("tools/call requires a tool name"))?;
442+
if name == "nightward_action_apply" {
443+
return Err(anyhow!(
444+
"nightward_action_apply is disabled in MCP because MCP clients cannot provide out-of-band local confirmation; use nightward_action_preview, then apply writes in the Nightward CLI, TUI, or Raycast extension"
445+
));
446+
}
459447
let args = validate_tool_args(
460448
name,
461449
params
@@ -611,25 +599,6 @@ fn call_tool_inner(params: Value, home: &Path) -> Result<Value> {
611599
}
612600
tool_result(sanitized_value(&actions::preview(home, &id)?)?)
613601
}
614-
"nightward_action_apply" => {
615-
let id = string_arg(&args, "action_id");
616-
if id.is_empty() {
617-
return Err(anyhow!("action_id is required"));
618-
}
619-
let result = actions::apply(
620-
home,
621-
&id,
622-
ApplyOptions {
623-
confirm: bool_arg(&args, "confirm", false),
624-
executable: string_arg(&args, "executable"),
625-
policy_path: string_arg(&args, "policy_path"),
626-
finding_id: string_arg(&args, "finding_id"),
627-
rule: string_arg(&args, "rule"),
628-
reason: string_arg(&args, "reason"),
629-
},
630-
)?;
631-
tool_result(sanitized_value(&result)?)
632-
}
633602
"nightward_rules" => tool_result(json!({
634603
"schema_version": 1,
635604
"rules": rules::all_rules()
@@ -643,7 +612,6 @@ fn call_tool_inner(params: Value, home: &Path) -> Result<Value> {
643612
enum ToolArgKind {
644613
String,
645614
Bool,
646-
ConfirmTrue,
647615
Limit,
648616
Severity,
649617
StringList,
@@ -712,13 +680,6 @@ fn validate_arg_value(tool: &str, spec: ToolArgSpec, value: &Value) -> Result<()
712680
Err(anyhow!("{tool} argument `{}` must be a boolean", spec.name))
713681
}
714682
}
715-
ToolArgKind::ConfirmTrue => {
716-
if value.as_bool() == Some(true) {
717-
Ok(())
718-
} else {
719-
Err(anyhow!("{tool} argument `{}` must be true", spec.name))
720-
}
721-
}
722683
ToolArgKind::Limit => {
723684
let Some(value) = value.as_u64() else {
724685
return Err(anyhow!(
@@ -830,15 +791,6 @@ fn tool_arg_specs(name: &str) -> Result<Vec<ToolArgSpec>> {
830791
ToolArgSpec::optional("head", String),
831792
],
832793
"nightward_action_preview" => vec![ToolArgSpec::required("action_id", String)],
833-
"nightward_action_apply" => vec![
834-
ToolArgSpec::required("action_id", String),
835-
ToolArgSpec::required("confirm", ConfirmTrue),
836-
ToolArgSpec::optional("executable", String),
837-
ToolArgSpec::optional("policy_path", String),
838-
ToolArgSpec::optional("finding_id", String),
839-
ToolArgSpec::optional("rule", String),
840-
ToolArgSpec::optional("reason", String),
841-
],
842794
_ => return Err(anyhow!("unknown tool {name}")),
843795
};
844796
Ok(specs)
@@ -1310,21 +1262,6 @@ fn schema_action_id() -> Value {
13101262
)
13111263
}
13121264

1313-
fn schema_action_apply() -> Value {
1314-
schema_object(
1315-
json!({
1316-
"action_id": { "type": "string" },
1317-
"confirm": { "type": "boolean", "const": true },
1318-
"executable": { "type": "string", "description": "Nightward executable path for schedule install actions." },
1319-
"policy_path": { "type": "string", "description": "Optional policy path under NIGHTWARD_HOME for policy actions." },
1320-
"finding_id": { "type": "string", "description": "Finding ID for policy.ignore." },
1321-
"rule": { "type": "string", "description": "Rule ID for policy.ignore." },
1322-
"reason": { "type": "string", "description": "Required reviewed reason for policy.ignore." }
1323-
}),
1324-
&["action_id", "confirm"],
1325-
)
1326-
}
1327-
13281265
#[cfg(test)]
13291266
mod tests {
13301267
use super::*;
@@ -1379,17 +1316,23 @@ mod tests {
13791316
"nightward_report_changes",
13801317
"nightward_actions_list",
13811318
"nightward_action_preview",
1382-
"nightward_action_apply",
13831319
"nightward_rules",
13841320
"nightward_providers",
13851321
] {
13861322
assert!(names.contains(name), "missing {name}");
13871323
}
1324+
assert!(!names.contains("nightward_action_apply"));
13881325
assert!(tools
13891326
.iter()
13901327
.all(|tool| tool["inputSchema"]["additionalProperties"] == false));
13911328
assert!(tools.iter().all(|tool| tool.get("outputSchema").is_some()));
13921329
assert!(tools.iter().all(|tool| tool.get("annotations").is_some()));
1330+
assert!(tools
1331+
.iter()
1332+
.all(|tool| tool["annotations"]["readOnlyHint"] == true));
1333+
assert!(tools
1334+
.iter()
1335+
.all(|tool| tool["annotations"]["destructiveHint"] == false));
13931336

13941337
let resources_response = handle_request_with_home(
13951338
json!({"jsonrpc":"2.0","id":2,"method":"resources/list"}),
@@ -1450,17 +1393,19 @@ mod tests {
14501393
}
14511394

14521395
#[test]
1453-
fn action_apply_is_disclosure_gated_tool_result_error() {
1396+
fn action_apply_is_disabled_and_cannot_accept_disclosure() {
14541397
let home = tempfile::tempdir().expect("temp home");
14551398
let response = handle_request_with_home(
1456-
json!({"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"nightward_action_apply","arguments":{"action_id":"backup.snapshot","confirm":true}}}),
1399+
json!({"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"nightward_action_apply","arguments":{"action_id":"disclosure.accept","confirm":true}}}),
14571400
home.path(),
14581401
);
14591402
assert_eq!(response["result"]["isError"], true);
14601403
assert!(response["result"]["content"][0]["text"]
14611404
.as_str()
14621405
.unwrap()
1463-
.contains("disclosure"));
1406+
.contains("disabled in MCP"));
1407+
assert!(!state::disclosure_status(home.path()).accepted);
1408+
assert!(!state::settings_path(home.path()).exists());
14641409
}
14651410

14661411
#[test]
@@ -1483,10 +1428,6 @@ mod tests {
14831428
json!({"name":"nightward_findings","arguments":{"severity":"urgent"}}),
14841429
"known severity",
14851430
),
1486-
(
1487-
json!({"name":"nightward_action_apply","arguments":{"action_id":"backup.snapshot","confirm":false}}),
1488-
"confirm` must be true",
1489-
),
14901431
] {
14911432
let response = handle_request_with_home(
14921433
json!({"jsonrpc":"2.0","id":1,"method":"tools/call","params":arguments}),
@@ -1567,7 +1508,7 @@ mod tests {
15671508
}
15681509

15691510
#[test]
1570-
fn action_apply_can_directly_apply_shared_registry_action() {
1511+
fn action_apply_remains_disabled_after_out_of_band_disclosure() {
15711512
let home = tempfile::tempdir().expect("temp home");
15721513
fs::create_dir_all(home.path().join(".codex")).expect("codex dir");
15731514
fs::write(home.path().join(".codex/config.toml"), "model = \"test\"\n").expect("config");
@@ -1587,15 +1528,12 @@ mod tests {
15871528
home.path(),
15881529
);
15891530

1590-
assert_eq!(response["result"]["isError"], false);
1591-
let writes = response["result"]["structuredContent"]["writes"]
1592-
.as_array()
1593-
.unwrap();
1594-
assert!(!writes.is_empty());
1595-
assert!(PathBuf::from(writes[0].as_str().unwrap())
1596-
.join(".codex/config.toml")
1597-
.is_file());
1598-
assert!(state::audit_path(home.path()).is_file());
1531+
assert_eq!(response["result"]["isError"], true);
1532+
assert!(response["result"]["content"][0]["text"]
1533+
.as_str()
1534+
.unwrap()
1535+
.contains("disabled in MCP"));
1536+
assert!(!state::state_dir(home.path()).join("snapshots").exists());
15991537
}
16001538

16011539
#[test]

0 commit comments

Comments
 (0)