Skip to content

fix(security): disable mcp action apply writes #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JSONbored merged 1 commit into from
May 6, 2026
Merged

fix(security): disable mcp action apply writes #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ The README uses a GIF so the preview renders directly on GitHub. The docs homepa
| --- | --- | --- |
| TUI | Dashboard, inventory, findings, analysis, fix plan, backup preview, action queue | Read-only until a confirmed action is applied |
| CLI | Scriptable scan, doctor, policy, SARIF, snapshot, schedule, backup, and action commands | Read-only unless explicit output/export paths or `--confirm` actions are requested |
| MCP server | Stdio tools/resources/prompts for AI clients | Direct apply only through shared action-registry IDs with disclosure, confirmation, redaction, and audit logging |
| MCP server | Stdio tools/resources/prompts for AI clients | Read-only; can list/preview actions, but writes must be applied in CLI/TUI/Raycast |
| Raycast | macOS companion commands plus confirmed Nightward Actions | Clipboard/report-folder actions plus confirmation-gated writes |
| GitHub Action | Workspace policy and SARIF checks | Writes only requested CI outputs |
| Trunk plugin | Local workspace policy/analyze linters | Emits SARIF to stdout |
Expand Down Expand Up @@ -296,7 +296,7 @@ Secret values are never emitted in scan JSON, findings output, fix-plan JSON, Ma

`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.

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.
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.

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.

Expand Down Expand Up @@ -356,7 +356,7 @@ Nightward can expose local context and bounded Nightward action workflows to MCP
}
```

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.
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.

## GitHub Action

Expand Down
120 changes: 29 additions & 91 deletions crates/nightward-core/src/mcpserver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::actions::{self, ApplyOptions};
use crate::actions;
use crate::analysis::{run as analyze, Options as AnalysisOptions};
use crate::fixplan::{plan as fix_plan, Selector};
use crate::inventory::{home_dir_from_env, load_report, redact_text, scan_home, scan_workspace};
Expand Down Expand Up @@ -97,7 +97,7 @@ fn initialize_result(requested: Option<&str>) -> Value {
"version": env!("CARGO_PKG_VERSION"),
"description": "Local-first AI agent, MCP, provider, and dotfiles security posture."
},
"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."
"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."
})
}

Expand Down Expand Up @@ -187,13 +187,6 @@ fn tools() -> Vec<Value> {
schema_action_id(),
read_only_annotations("Action preview", false),
),
tool(
"nightward_action_apply",
"Action Apply",
"Apply one bounded Nightward action after disclosure acceptance and confirm: true.",
schema_action_apply(),
write_annotations("Action apply", true, true),
),
tool(
"nightward_rules",
"Nightward Rules",
Expand Down Expand Up @@ -349,16 +342,6 @@ fn read_only_annotations(title: &str, open_world: bool) -> Value {
})
}

fn write_annotations(title: &str, destructive: bool, open_world: bool) -> Value {
json!({
"title": title,
"readOnlyHint": false,
"destructiveHint": destructive,
"idempotentHint": false,
"openWorldHint": open_world
})
}

fn read_resource(params: Value, home: &Path) -> Result<Value> {
let uri = params
.get("uri")
Expand Down Expand Up @@ -401,7 +384,7 @@ fn read_prompt(params: Value) -> Result<Value> {
let finding_id = string_arg(&args, "finding_id");
let text = match name {
"audit_my_ai_setup" => {
"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."
"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."
}
"explain_top_risks" => {
"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."
Expand All @@ -410,7 +393,7 @@ fn read_prompt(params: Value) -> Result<Value> {
return Ok(prompt_result(
"Generate a safe Nightward fix workflow.",
format!(
"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.",
"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.",
if finding_id.is_empty() {
"<finding-id>"
} else {
Expand All @@ -420,7 +403,7 @@ fn read_prompt(params: Value) -> Result<Value> {
));
}
"set_up_providers" => {
"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."
"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."
}
"compare_reports" => {
"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."
Expand Down Expand Up @@ -456,6 +439,11 @@ fn call_tool_inner(params: Value, home: &Path) -> Result<Value> {
.get("name")
.and_then(Value::as_str)
.ok_or_else(|| anyhow!("tools/call requires a tool name"))?;
if name == "nightward_action_apply" {
return Err(anyhow!(
"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"
));
}
let args = validate_tool_args(
name,
params
Expand Down Expand Up @@ -611,25 +599,6 @@ fn call_tool_inner(params: Value, home: &Path) -> Result<Value> {
}
tool_result(sanitized_value(&actions::preview(home, &id)?)?)
}
"nightward_action_apply" => {
let id = string_arg(&args, "action_id");
if id.is_empty() {
return Err(anyhow!("action_id is required"));
}
let result = actions::apply(
home,
&id,
ApplyOptions {
confirm: bool_arg(&args, "confirm", false),
executable: string_arg(&args, "executable"),
policy_path: string_arg(&args, "policy_path"),
finding_id: string_arg(&args, "finding_id"),
rule: string_arg(&args, "rule"),
reason: string_arg(&args, "reason"),
},
)?;
tool_result(sanitized_value(&result)?)
}
"nightward_rules" => tool_result(json!({
"schema_version": 1,
"rules": rules::all_rules()
Expand All @@ -643,7 +612,6 @@ fn call_tool_inner(params: Value, home: &Path) -> Result<Value> {
enum ToolArgKind {
String,
Bool,
ConfirmTrue,
Limit,
Severity,
StringList,
Expand Down Expand Up @@ -712,13 +680,6 @@ fn validate_arg_value(tool: &str, spec: ToolArgSpec, value: &Value) -> Result<()
Err(anyhow!("{tool} argument `{}` must be a boolean", spec.name))
}
}
ToolArgKind::ConfirmTrue => {
if value.as_bool() == Some(true) {
Ok(())
} else {
Err(anyhow!("{tool} argument `{}` must be true", spec.name))
}
}
ToolArgKind::Limit => {
let Some(value) = value.as_u64() else {
return Err(anyhow!(
Expand Down Expand Up @@ -830,15 +791,6 @@ fn tool_arg_specs(name: &str) -> Result<Vec<ToolArgSpec>> {
ToolArgSpec::optional("head", String),
],
"nightward_action_preview" => vec![ToolArgSpec::required("action_id", String)],
"nightward_action_apply" => vec![
ToolArgSpec::required("action_id", String),
ToolArgSpec::required("confirm", ConfirmTrue),
ToolArgSpec::optional("executable", String),
ToolArgSpec::optional("policy_path", String),
ToolArgSpec::optional("finding_id", String),
ToolArgSpec::optional("rule", String),
ToolArgSpec::optional("reason", String),
],
_ => return Err(anyhow!("unknown tool {name}")),
};
Ok(specs)
Expand Down Expand Up @@ -1310,21 +1262,6 @@ fn schema_action_id() -> Value {
)
}

fn schema_action_apply() -> Value {
schema_object(
json!({
"action_id": { "type": "string" },
"confirm": { "type": "boolean", "const": true },
"executable": { "type": "string", "description": "Nightward executable path for schedule install actions." },
"policy_path": { "type": "string", "description": "Optional policy path under NIGHTWARD_HOME for policy actions." },
"finding_id": { "type": "string", "description": "Finding ID for policy.ignore." },
"rule": { "type": "string", "description": "Rule ID for policy.ignore." },
"reason": { "type": "string", "description": "Required reviewed reason for policy.ignore." }
}),
&["action_id", "confirm"],
)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -1379,17 +1316,23 @@ mod tests {
"nightward_report_changes",
"nightward_actions_list",
"nightward_action_preview",
"nightward_action_apply",
"nightward_rules",
"nightward_providers",
] {
assert!(names.contains(name), "missing {name}");
}
assert!(!names.contains("nightward_action_apply"));
assert!(tools
.iter()
.all(|tool| tool["inputSchema"]["additionalProperties"] == false));
assert!(tools.iter().all(|tool| tool.get("outputSchema").is_some()));
assert!(tools.iter().all(|tool| tool.get("annotations").is_some()));
assert!(tools
.iter()
.all(|tool| tool["annotations"]["readOnlyHint"] == true));
assert!(tools
.iter()
.all(|tool| tool["annotations"]["destructiveHint"] == false));

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

#[test]
fn action_apply_is_disclosure_gated_tool_result_error() {
fn action_apply_is_disabled_and_cannot_accept_disclosure() {
let home = tempfile::tempdir().expect("temp home");
let response = handle_request_with_home(
json!({"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"nightward_action_apply","arguments":{"action_id":"backup.snapshot","confirm":true}}}),
json!({"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"nightward_action_apply","arguments":{"action_id":"disclosure.accept","confirm":true}}}),
home.path(),
);
assert_eq!(response["result"]["isError"], true);
assert!(response["result"]["content"][0]["text"]
.as_str()
.unwrap()
.contains("disclosure"));
.contains("disabled in MCP"));
assert!(!state::disclosure_status(home.path()).accepted);
assert!(!state::settings_path(home.path()).exists());
}

#[test]
Expand All @@ -1483,10 +1428,6 @@ mod tests {
json!({"name":"nightward_findings","arguments":{"severity":"urgent"}}),
"known severity",
),
(
json!({"name":"nightward_action_apply","arguments":{"action_id":"backup.snapshot","confirm":false}}),
"confirm` must be true",
),
] {
let response = handle_request_with_home(
json!({"jsonrpc":"2.0","id":1,"method":"tools/call","params":arguments}),
Expand Down Expand Up @@ -1567,7 +1508,7 @@ mod tests {
}

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

assert_eq!(response["result"]["isError"], false);
let writes = response["result"]["structuredContent"]["writes"]
.as_array()
.unwrap();
assert!(!writes.is_empty());
assert!(PathBuf::from(writes[0].as_str().unwrap())
.join(".codex/config.toml")
.is_file());
assert!(state::audit_path(home.path()).is_file());
assert_eq!(response["result"]["isError"], true);
assert!(response["result"]["content"][0]["text"]
.as_str()
.unwrap()
.contains("disabled in MCP"));
assert!(!state::state_dir(home.path()).join("snapshots").exists());
}

#[test]
Expand Down
Loading
Loading