Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0306f26
refactor(security): unify policy engine on Capability + Provenance
xinhuagu May 17, 2026
7f6a854
feat(mcp): migrate McpToolBridge to CapabilityAware
xinhuagu May 17, 2026
511f30c
fix(security): run structural denials before session-blanket lookup
xinhuagu May 17, 2026
22fdab7
fix(mcp): infer FileWrite/FileDelete at MCP boundary so structural ru…
xinhuagu May 17, 2026
0b9acbb
fix(security): match sensitive paths case-insensitively
xinhuagu May 17, 2026
f813df0
fix(mcp): infer FileWrite/FileDelete for move/rename/copy operations
xinhuagu May 17, 2026
627392a
fix(mcp): deny moves that remove sensitive sources
xinhuagu May 17, 2026
3dedaa2
fix(mcp): preserve DANGEROUS risk on moves; tighten verb regex + null…
xinhuagu May 17, 2026
7cfd3cb
fix(mcp): match path field names case-insensitive and snake/camel-agn…
xinhuagu May 17, 2026
a00b9ab
fix(mcp): treat copies-from-sensitive-source as exfiltration
xinhuagu May 18, 2026
c771e7f
fix(security): route sub-agent dispatch through structural denials
xinhuagu May 18, 2026
f7e8b2b
fix(mcp): normalize method names so camelCase/kebab/PascalCase tools …
xinhuagu May 18, 2026
f77a13f
fix(security): audit sub-agent structural denials; tighten review fin…
xinhuagu May 18, 2026
36b86e3
test(security): cover null-provenance denial branch on checkStructural
xinhuagu May 18, 2026
88c0d97
refactor(mcp): extract McpCapabilityInference out of McpToolBridge
xinhuagu May 18, 2026
3834ce9
refactor(security): extract SensitivePaths utility, add Capability.Fi…
xinhuagu May 18, 2026
eba2232
fix(security): make /etc/ rule OS-independent for Windows CI
xinhuagu May 18, 2026
1f2b4d7
test(watchdog): de-flake disabledTurnBudget_onlyTimeApplies on CI
xinhuagu May 18, 2026
0007629
fix(security): close sub-agent structural-bypass holes per #495 review
xinhuagu May 23, 2026
29b7a48
test(memory): de-bomb CandidateStore migration test against retention
xinhuagu May 23, 2026
5e4a1c2
fix(security): resolve symlinks before sensitive-path match
xinhuagu May 23, 2026
56db109
build(tools): flip aceclaw-security to api() scope to match public su…
xinhuagu May 23, 2026
d13703f
Revert "fix(security): resolve symlinks before sensitive-path match"
xinhuagu May 23, 2026
c8b1e00
feat(security): gate structural sensitive-path denials behind opt-in …
xinhuagu May 23, 2026
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
Prev Previous commit
Next Next commit
fix(mcp): treat copies-from-sensitive-source as exfiltration
Codex P1 on #495 (round 9): copy_file(source=".env", destination="/tmp/x")
read a credentials file and duplicated its contents into a non-sensitive
location. With my round-7 model, copy emitted FileWrite(dst) - WRITE risk
- which auto-approves in accept-edits mode. Pre-#495 MCP tools prompted
via the EXECUTE fallback, so this was a regression specifically
introduced by the CapabilityAware migration.

Re-classify copies-from-sensitive-source as FileDelete(src). The op
doesn't actually delete the source; the classification is intentionally
over-conservative so the structural denial layer refuses the operation
regardless of where it lands. Audit log shows @type=FileDelete for what
is technically a read+write; the toolName field still carries
mcp__<server>__copy_file so operators can correlate.

Unified disambiguation order for move/copy ops:

  1. dst sensitive                    -> FileWrite(dst), denied
  2. src present AND (isMove OR
     src sensitive)                   -> FileDelete(src)
        - moves (any src): DANGEROUS prompt for the delete
        - copies from sensitive src: structurally denied as exfil
  3. dst present (safe + safe copy)   -> FileWrite(dst), WRITE risk OK

Two existing tests retargeted - their old assertions pinned the wrong
behaviour:
- copyFromSensitiveSourceDoesNotInferDelete (was: expect McpInvoke)
  -> copyFromSensitiveSourceWithoutDestinationInfersFileDelete
- copyFromSensitiveSourceWithSafeDestinationStaysAsFileWrite
  -> copyFromSensitiveSourceToSafeDestinationIsStructurallyDeniedAsExfil
  (plus an end-to-end assertion that DefaultPermissionPolicy denies the
  resulting FileDelete in auto-accept mode)

38 tests pass in McpToolBridgeTest. Full build green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
  • Loading branch information
xinhuagu and claude committed May 18, 2026
commit a00b9ab1a1bdad9a1e66b120ed54557287c14110
40 changes: 24 additions & 16 deletions aceclaw-mcp/src/main/java/dev/aceclaw/mcp/McpToolBridge.java
Original file line number Diff line number Diff line change
Expand Up @@ -254,30 +254,38 @@ private Capability inferFileCapability(JsonNode args) {
// "delete" target (for moves only β€” copies leave the source).
// Disambiguation order:
// 1. dst is sensitive β†’ FileWrite(dst). Catches "move/copy to
// .env" β€” destination-write attack.
// 2. Op is a move (not copy) and src resolves β†’ FileDelete(src).
// Two purposes:
// a. Catches "move .env away" β€” source-delete attack the
// destination-first model missed (Codex P1 follow-up #2
// .env" β€” the destination-write attack.
// 2. src resolves AND (op is a move OR src is sensitive) β†’
// FileDelete(src). Two distinct purposes folded together:
// a. For MOVES (regardless of src sensitivity): the
// source really is removed, and FileDelete's DANGEROUS
// risk prompts in accept-edits where FileWrite would
// auto-approve. Pre-#495 MCP moves prompted via
// EXECUTE risk; this preserves that floor (Codex P2
// on #495).
// b. Preserves DANGEROUS risk on the move semantics so even
// benign moves don't auto-approve in accept-edits mode.
// Pre-#495 MCP moves prompted via EXECUTE risk; FileWrite
// is WRITE which IS auto-approved in accept-edits β€” that
// would silently delete the source. FileDelete is
// DANGEROUS, which is the closest match (Codex P2
// on #495).
// 3. Pure copies (no source side-effect) β†’ FileWrite(dst). Risk
// is the genuine WRITE risk; auto-approval in accept-edits
// is fine because the source is intact.
// b. For COPIES from a sensitive source: copies don't
// actually delete the source, but duplicating a
// credentials file to a non-sensitive location IS
// exfiltration. Pre-#495 the same call prompted via
// the EXECUTE fallback; emitting FileDelete here
// triggers the structural denial via the sensitive
// source path (Codex P1 on #495 β€” "preserve prompts
// for copies from sensitive sources"). Audit log
// shows @type=FileDelete for a copy β€” slightly
// misleading but safer than missing the denial; the
// toolName field still carries
// mcp__<server>__copy_file so operators can correlate.
// 3. Pure copy with non-sensitive src (dst is safe per step 1)
// β†’ FileWrite(dst). Genuine WRITE risk; auto-approval in
// accept-edits is fine.
Path dst = safePath(extractField(args, DESTINATION_FIELDS));
Path src = safePath(extractField(args, SOURCE_FIELDS));
boolean isMove = !COPY_VERB.matcher(name).matches();

if (dst != null && DefaultPermissionPolicy.isSensitivePath(dst)) {
return new Capability.FileWrite(dst, WriteMode.OVERWRITE);
}
if (isMove && src != null) {
if (src != null && (isMove || DefaultPermissionPolicy.isSensitivePath(src))) {
return new Capability.FileDelete(src);
}
if (dst != null) return new Capability.FileWrite(dst, WriteMode.OVERWRITE);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve dangerous risk for MCP moves

When an MCP move_file/rename has a destination and both paths are non-sensitive, this emits FileWrite, whose WRITE risk is auto-approved in accept-edits mode (DefaultPermissionPolicy approves READ/WRITE). I checked the dispatcher path: MCP tools used to default to EXECUTE in StreamingAgentHandler, so the same move previously prompted; now move_file(source="/tmp/a", destination="/tmp/b") can silently remove the source even though Capability.FileDelete is intentionally DANGEROUS to avoid auto-approving deletions. Model non-copy moves as requiring the delete-side risk, or otherwise keep them at MCP/execute risk when they remove a source.

Useful? React with πŸ‘Β / πŸ‘Ž.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve prompts for copies from sensitive sources

When the MCP method is a copy and the destination is non-sensitive, this returns FileWrite(dst) even if src is /repo/.env or ~/.ssh/config. Because DefaultPermissionPolicy auto-approves WRITE in accept-edits mode and the structural check only sees the safe destination, copy_file(source="/repo/.env", destination="/tmp/env-copy.txt") can run without a prompt and duplicate credentials into a non-sensitive path; before this adapter became CapabilityAware, the same MCP call used the EXECUTE fallback and prompted. Copies do not delete the source, but they still read and disclose its contents, so sensitive sources need to retain at least the MCP/execute prompt or be modeled with a source-side capability.

Useful? React with πŸ‘Β / πŸ‘Ž.

Expand Down
37 changes: 25 additions & 12 deletions aceclaw-mcp/src/test/java/dev/aceclaw/mcp/McpToolBridgeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -305,17 +305,22 @@ void moveFromSensitiveSourceInfersFileDelete() {
}

@Test
void copyFromSensitiveSourceDoesNotInferDelete() {
// Copies leave the source intact β€” no FileDelete inferred.
void copyFromSensitiveSourceWithoutDestinationInfersFileDelete() {
// Codex P1 follow-up on #495 (round-9): copies from sensitive sources
// re-classify as FileDelete(src) so the structural denial fires --
// duplicating credentials anywhere is exfiltration regardless of
// destination. The op doesn't actually delete the source; the
// classification is intentionally over-conservative (audit log shows
// FileDelete but toolName=mcp__<server>__copy_file keeps it
// traceable).
var tool = McpToolBridge.create("fs", mcpTool("copy_file", "desc"), client);

var args = new ObjectMapper().createObjectNode()
.put("source", "/repo/.env");
var cap = tool.toCapability(args);

// No destination resolved; not a delete (copy doesn't delete source).
// Falls through to McpInvoke.
assertThat(cap).isInstanceOf(Capability.McpInvoke.class);
assertThat(cap).isInstanceOf(Capability.FileDelete.class);
assertThat(((Capability.FileDelete) cap).path()).isEqualTo(Path.of("/repo/.env"));
}

@Test
Expand All @@ -339,20 +344,28 @@ void moveFromSensitiveSourceWithSafeDestinationInfersFileDelete() {
}

@Test
void copyFromSensitiveSourceWithSafeDestinationStaysAsFileWrite() {
// Copies don't delete the source - the source-sensitivity probe only
// kicks in for moves. A copy from .env to safe-dest still emits
// FileWrite(safe-dest) (the destination side), gets the standard
// prompt - no source-side denial because the source is left intact.
void copyFromSensitiveSourceToSafeDestinationIsStructurallyDeniedAsExfil() {
// Codex P1 on #495 (round-9): copy_file(.env, /tmp/env-copy) reads
// sensitive content and duplicates it to a non-sensitive location --
// a credential-exfiltration vector. Pre-fix this auto-approved in
// accept-edits because FileWrite(safe-dst) has WRITE risk. Re-classify
// copies-from-sensitive-src as FileDelete(src) so the structural
// denial layer refuses them regardless of destination, matching the
// pre-#495 EXECUTE-prompt behaviour for MCP tools.
var tool = McpToolBridge.create("fs", mcpTool("copy_file", "desc"), client);

var args = new ObjectMapper().createObjectNode()
.put("source", "/repo/.env")
.put("destination", "/tmp/env-copy.txt");
var cap = tool.toCapability(args);

assertThat(cap).isInstanceOf(Capability.FileWrite.class);
assertThat(((Capability.FileWrite) cap).path()).isEqualTo(Path.of("/tmp/env-copy.txt"));
assertThat(cap)
.as("copy from sensitive src must be classified as FileDelete to trigger denial")
.isInstanceOf(Capability.FileDelete.class);
var decision = new DefaultPermissionPolicy("auto-accept").evaluateStructural(cap);
assertThat(decision)
.as("structural denial fires even in auto-accept mode")
.isNotNull();
}

@Test
Expand Down
Loading