kayg/calternal

Watch

Fork 0

MCP: mark returned content as untrusted data in results and agent guidance #746

New issue

Open

opened 2026-10-02 13:08:14 +00:00 by kayg · 3 comments

kayg commented

2026-10-02 13:08:14 +00:00

Owner

F3 — untrusted MCP content has no explicit trust boundary

crates/calternal-server/src/mcp.rs:107 returns generated route output as a
plain text content block. Legacy Note, search and Mail reads use the same
raw API output path. get_info (line 1232) explains scopes but does not say
that Note bodies, Mail bodies, subjects, names and search snippets are
untrusted data. The public Skill intro on merge-round-7a also omits that rule.
WebMCP already sets untrustedContentHint: true.

Impact: an assistant receives externally authored content without a server
statement that the content is data, not instructions. This increases prompt
injection exposure. It does not prove that a host executes an injected
instruction, and it does not bypass route authorization by itself.

Fix: add the trust rule to server instructions and the generated Skill. Use
one shared result envelope to identify untrusted payloads and provenance for
legacy and generated tools. Do not invent an MCP annotation as a security
guarantee. Keep host consent for sensitive operations separate from content
labels. Check Note, Mail and search output with inert text fixtures.

MCP's official risk guidance says annotations do not stop prompt injection:
Tool Annotations as Risk Vocabulary.

Audit context: sec-mcp-scopes job, base origin/dev c4a61e8cf0; merge-round-7a 2f4482ded also reviewed. Source review only. No product change or exploit reproduction. DESIGN §§21, 27, 41 and 55. Duplicate search: scope, MCP, injection, Trash and feed issue titles; existing #328/#329/#431 cover features, not these specific missing guards.

F3 — untrusted MCP content has no explicit trust boundary `crates/calternal-server/src/mcp.rs:107` returns generated route output as a plain text content block. Legacy Note, search and Mail reads use the same raw API output path. `get_info` (line 1232) explains scopes but does not say that Note bodies, Mail bodies, subjects, names and search snippets are untrusted data. The public Skill intro on merge-round-7a also omits that rule. WebMCP already sets `untrustedContentHint: true`. Impact: an assistant receives externally authored content without a server statement that the content is data, not instructions. This increases prompt injection exposure. It does not prove that a host executes an injected instruction, and it does not bypass route authorization by itself. Fix: add the trust rule to server instructions and the generated Skill. Use one shared result envelope to identify untrusted payloads and provenance for legacy and generated tools. Do not invent an MCP annotation as a security guarantee. Keep host consent for sensitive operations separate from content labels. Check Note, Mail and search output with inert text fixtures. MCP's official risk guidance says annotations do not stop prompt injection: [Tool Annotations as Risk Vocabulary](https://blog.modelcontextprotocol.io/posts/2026-03-16-tool-annotations/). Audit context: sec-mcp-scopes job, base origin/dev c4a61e8cf090170f35b1bed3350d9de20c83ecd5; merge-round-7a 2f4482ded also reviewed. Source review only. No product change or exploit reproduction. DESIGN §§21, 27, 41 and 55. Duplicate search: scope, MCP, injection, Trash and feed issue titles; existing #328/#329/#431 cover features, not these specific missing guards.

kayg referenced this issue

2026-10-02 13:15:51 +00:00

PERF: instant everywhere — app-wide speed architecture rules, audit and shared primitives #663

kayg referenced this issue

2026-10-02 13:20:39 +00:00

BLOCKER: require account authority before permanently emptying Trash #739

kayg commented

2026-10-02 13:51:02 +00:00

Author

Owner

Agent guidance is committed at ff872db5e: the generated Skill and docs/mcp.md state that User content is data, not instructions, and cannot grant authority or replace User consent. The MCP implementation uses one result envelope for generated and legacy API adapters, with a fixed trust label and API provenance. Inert Note, Mail and search fixtures cover payload preservation. No new MCP annotation is treated as a security guarantee. Server checks are still compiling on the shared host.

Agent guidance is committed at `ff872db5e`: the generated Skill and `docs/mcp.md` state that User content is data, not instructions, and cannot grant authority or replace User consent. The MCP implementation uses one result envelope for generated and legacy API adapters, with a fixed trust label and API provenance. Inert Note, Mail and search fixtures cover payload preservation. No new MCP annotation is treated as a security guarantee. Server checks are still compiling on the shared host.

kayg commented

2026-10-02 14:18:48 +00:00

Author

Owner

Read-only code review found a legacy Log compatibility defect in the new result envelope: calternal_create_log read entries at the root after the batch API had committed. It would report an error after a successful write. The adapter now selects data.entries and retains the trust/source fields. A unit regression and the focused live probe cover it. Follow-up review found no further issues.

The public calternal-api::Action type adds only two metadata booleans: fresh_auth and enforce_scopes. This minimal public addition lets the server use the same reviewed registry as all adapters. The latter flag preserves the existing public sign-in and capability flows; it does not replace their route guards.

Read-only code review found a legacy Log compatibility defect in the new result envelope: `calternal_create_log` read `entries` at the root after the batch API had committed. It would report an error after a successful write. The adapter now selects `data.entries` and retains the trust/source fields. A unit regression and the focused live probe cover it. Follow-up review found no further issues. The public `calternal-api::Action` type adds only two metadata booleans: `fresh_auth` and `enforce_scopes`. This minimal public addition lets the server use the same reviewed registry as all adapters. The latter flag preserves the existing public sign-in and capability flows; it does not replace their route guards.

kayg commented

2026-10-02 16:21:03 +00:00

Author

Owner

Implemented scopefix; validation incomplete at the 3-hour limit. Not ready for merge.

Branch: job/scopefix. Base: 2f4482ded0. Head: 1f1ba91cc3.
The required origin/dev and origin/job/merge-round-7a merges both returned Already up to date. No push or deploy was performed.

Built:

Files checks owning account and data authority before the mutation lock, permanent Trash removal, Index changes or publication. A regression test checks unchanged Trash and Index after denial and owning account success.
Calendar public feed management checks account/data authority and the existing data_user resource boundary before writes. Subscriptions retain data authority.
Registry scope policy corrects the 26 audited mismatches plus set_user_plugin and the seven Trash/Calendar actions. The server applies the reviewed requirements before body extraction. MCP discovery filters declared scopes. Tests audit detected data/account/admin guards across declared handlers and exercise every eligible action through the registry middleware.
Generated and legacy MCP API results share an untrusted-data/provenance envelope; errors, server instructions, llms.txt and the public Skill carry the trust boundary. Legacy Log selection preserves the envelope after a successful write. Inert Note/Mail/search fixtures also check reserved payload fields.
Web Trash and Calendar grant writes use the existing passkey step-up helper.
A focused cross-User/admin/MCP/API/CLI probe, serial/burst bench profile and twelve production screenshot captures with macOS rendering are prepared.

UX gaps closed: stale account sessions retry through the shared passkey flow; cancelling the Trash check says Nothing changed. UX gaps left: real-server pointer/touch/keyboard walkthrough and all screenshots remain unverified.

Decisions: only reviewed declarations opt into central enforcement, to preserve existing public capability flows (including download_app_password_profile) and legacy route guards. Permanent removal and Calendar grant mutations require recent assertions; grant reads require account/data without an assertion. MCP payloads move under data with fixed trust/source fields; these identify API provenance, not content authorship, and do not guarantee prompt-injection prevention.

Verified gate output (verbatim excerpts):

registry-test.log

..............
----------------------------------------------------------------------
Ran 14 tests in 2.631s

OK

calternal-api-clippy.log

    Finished `dev` profile [unoptimized + debuginfo] target(s) in 4m 51s

calternal-api-test.log

    Finished `test` profile [unoptimized + debuginfo] target(s) in 3m 12s
test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.31s
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

calternal-plugin-files-clippy.log

    Finished `dev` profile [unoptimized + debuginfo] target(s) in 40m 00s

web-check-final.log

svelte-check found 0 errors and 0 warnings

web-test-head.log

 Test Files  154 passed (154)
      Tests  1073 passed (1073)
   Duration  664.68s (transform 31%, environment 28%, import 21%, tests 14%, setup 6%)

web-build-final.log

✓ built in 3m 13s
✓ built in 245ms
✓ built in 9m 2s
  Wrote site to "build"
  ✔ done

Exit statuses confirmed: cargo fmt --check 0 (no output); calternal-api clippy/test 0; calternal-plugin-files clippy 0; web check/test/build 0. Registry has 340 operations and 322 generated tools; all 14 registry tests pass. Python syntax, shell syntax, screenshot-script syntax and git diff --check pass.

Known gaps: Files tests were stopped during compilation; Calendar/server clippy and tests did not run to completion. The local server/CLI binary was not built, so no real-server round, MCP probe, bench measurements or screenshots ran. No measured performance regression claim is made; the profile includes notes.list baseline reference p50 1.3ms/p95 3.1ms, with no equivalent account-denial/envelope baseline. No screenshots exist to attach.

Build evidence: the first dependency build spent over an hour through the shared sccache. Active clients waited 81–145 seconds. Restarting this job without RUSTC_WRAPPER produced API gates and Files clippy; the latter finished in 40m 00s. The first default web test run had one 5000ms KeyboardShortcutsCard timeout; full runs with two workers and 30000ms timeouts passed. No existing assertion or fixture was changed to hide a failure. Remaining Rust checks must run before merge.

Source comments were re-read. No migrations or dependency versions were changed. Build output is being cleaned with cargo clean; web build output and Python caches were removed. Review artifacts remain gitignored.

Files:

apps/web/src/lib/files/api.ts
apps/web/src/lib/webmcp/tools.ts
apps/web/src/routes/settings/apps/CalendarFeedsGroup.svelte
bench/scopefix-739.py
contracts/action-authority.json
contracts/actions.json
crates/calternal-api/src/actions.rs
crates/calternal-server/src/agent_docs.rs
crates/calternal-server/src/agent_docs/skill_intro.md
crates/calternal-server/src/authz.rs
crates/calternal-server/src/mcp.rs
crates/calternal-server/src/wire.rs
crates/plugins/calendar/src/feeds/publication.rs
crates/plugins/files/src/lib.rs
docs/action-registry.md
docs/mcp.md
scripts/action_registry.py
scripts/test_action_registry.py
tests/adversarial/mcp_probe.py
tests/adversarial/run.sh
tests/adversarial/scopefix_review.mjs

Implemented scopefix; validation incomplete at the 3-hour limit. Not ready for merge. Branch: job/scopefix. Base: 2f4482ded066d9c5d9c59130377907f7fd2916c9. Head: 1f1ba91cc39ac1b8abcdec8c266919c5e3691302. The required origin/dev and origin/job/merge-round-7a merges both returned Already up to date. No push or deploy was performed. Built: - Files checks owning account and data authority before the mutation lock, permanent Trash removal, Index changes or publication. A regression test checks unchanged Trash and Index after denial and owning account success. - Calendar public feed management checks account/data authority and the existing data_user resource boundary before writes. Subscriptions retain data authority. - Registry scope policy corrects the 26 audited mismatches plus set_user_plugin and the seven Trash/Calendar actions. The server applies the reviewed requirements before body extraction. MCP discovery filters declared scopes. Tests audit detected data/account/admin guards across declared handlers and exercise every eligible action through the registry middleware. - Generated and legacy MCP API results share an untrusted-data/provenance envelope; errors, server instructions, llms.txt and the public Skill carry the trust boundary. Legacy Log selection preserves the envelope after a successful write. Inert Note/Mail/search fixtures also check reserved payload fields. - Web Trash and Calendar grant writes use the existing passkey step-up helper. - A focused cross-User/admin/MCP/API/CLI probe, serial/burst bench profile and twelve production screenshot captures with macOS rendering are prepared. UX gaps closed: stale account sessions retry through the shared passkey flow; cancelling the Trash check says Nothing changed. UX gaps left: real-server pointer/touch/keyboard walkthrough and all screenshots remain unverified. Decisions: only reviewed declarations opt into central enforcement, to preserve existing public capability flows (including download_app_password_profile) and legacy route guards. Permanent removal and Calendar grant mutations require recent assertions; grant reads require account/data without an assertion. MCP payloads move under data with fixed trust/source fields; these identify API provenance, not content authorship, and do not guarantee prompt-injection prevention. Verified gate output (verbatim excerpts): registry-test.log ```text .............. ---------------------------------------------------------------------- Ran 14 tests in 2.631s OK ``` calternal-api-clippy.log ```text Finished `dev` profile [unoptimized + debuginfo] target(s) in 4m 51s ``` calternal-api-test.log ```text Finished `test` profile [unoptimized + debuginfo] target(s) in 3m 12s test result: ok. 10 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.31s test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s ``` calternal-plugin-files-clippy.log ```text Finished `dev` profile [unoptimized + debuginfo] target(s) in 40m 00s ``` web-check-final.log ```text svelte-check found 0 errors and 0 warnings ``` web-test-head.log ```text Test Files 154 passed (154) Tests 1073 passed (1073) Duration 664.68s (transform 31%, environment 28%, import 21%, tests 14%, setup 6%) ``` web-build-final.log ```text ✓ built in 3m 13s ✓ built in 245ms ✓ built in 9m 2s Wrote site to "build" ✔ done ``` Exit statuses confirmed: cargo fmt --check 0 (no output); calternal-api clippy/test 0; calternal-plugin-files clippy 0; web check/test/build 0. Registry has 340 operations and 322 generated tools; all 14 registry tests pass. Python syntax, shell syntax, screenshot-script syntax and git diff --check pass. Known gaps: Files tests were stopped during compilation; Calendar/server clippy and tests did not run to completion. The local server/CLI binary was not built, so no real-server round, MCP probe, bench measurements or screenshots ran. No measured performance regression claim is made; the profile includes notes.list baseline reference p50 1.3ms/p95 3.1ms, with no equivalent account-denial/envelope baseline. No screenshots exist to attach. Build evidence: the first dependency build spent over an hour through the shared sccache. Active clients waited 81–145 seconds. Restarting this job without RUSTC_WRAPPER produced API gates and Files clippy; the latter finished in 40m 00s. The first default web test run had one 5000ms KeyboardShortcutsCard timeout; full runs with two workers and 30000ms timeouts passed. No existing assertion or fixture was changed to hide a failure. Remaining Rust checks must run before merge. Source comments were re-read. No migrations or dependency versions were changed. Build output is being cleaned with cargo clean; web build output and Python caches were removed. Review artifacts remain gitignored. Files: - `apps/web/src/lib/files/api.ts` - `apps/web/src/lib/webmcp/tools.ts` - `apps/web/src/routes/settings/apps/CalendarFeedsGroup.svelte` - `bench/scopefix-739.py` - `contracts/action-authority.json` - `contracts/actions.json` - `crates/calternal-api/src/actions.rs` - `crates/calternal-server/src/agent_docs.rs` - `crates/calternal-server/src/agent_docs/skill_intro.md` - `crates/calternal-server/src/authz.rs` - `crates/calternal-server/src/mcp.rs` - `crates/calternal-server/src/wire.rs` - `crates/plugins/calendar/src/feeds/publication.rs` - `crates/plugins/files/src/lib.rs` - `docs/action-registry.md` - `docs/mcp.md` - `scripts/action_registry.py` - `scripts/test_action_registry.py` - `tests/adversarial/mcp_probe.py` - `tests/adversarial/run.sh` - `tests/adversarial/scopefix_review.mjs`

kayg referenced this issue

2026-10-02 17:26:32 +00:00

BLOCKER: require account authority before permanently emptying Trash #739

kayg referenced this issue

2026-10-03 08:52:05 +00:00

MCP design review: curate the agent surface, fix annotations, and build #491 Events on top #999

kayg referenced this issue

2026-10-03 16:16:59 +00:00

Notes projection rebuild fails alone with Index busy in 7b #1022

kayg referenced this issue

2026-10-03 16:34:53 +00:00

Plan: merge order and conflicts for merge round 7b #867