fix: scope MCP and the ledger listing by token claims; MCP follows data auth - #1963

🟠 HIGH-1 — /events?all=true is the same enumeration hole /ledgers just had, and it isn't closed

(in-diff range 578-591).

The PR's premise for this route is "It listed every ledger and graph source even with data auth required." That sentence is still true, verbatim, of GET /v1/fluree/events?all=true — build_initial_snapshot (routes/events.rs:344-362) walks the same all_records() and all_graph_source_records() and emits an ns-record event per ledger.

The reason it isn't covered is that /events is gated by events_auth, a separate config axis. EventsAuthMode defaults to None (config.rs:41-44) and --data-auth-mode required does not touch it, so the two ends up disagreeing on the same server. I ran both against one instance with data_auth_mode: Required:

GET /v1/fluree/ledgers            (no token)  -> 401 {"error":"Bearer token required"}
GET /v1/fluree/ledgers            (alpha tok) -> 200 [{"name":"alpha",...}]          # only alpha
GET /v1/fluree/events?all=true    (no token)  -> 200
event: ns-record  data: {"resource_id":"alpha:main","record":{...}}
event: ns-record  data: {"resource_id":"beta:main","record":{...}}

To be clear on attribution: this PR doesn't introduce it, and filter_to_allowed (events.rs:477-518) is correct on its own terms — it expands all=true down to the token's own set rather than passing it through, which is the right fail-closed shape once events auth is on. The problem is only that an operator who turns on data auth reasonably believes they have turned on ledger enumeration control, and the Behavior-changes section of this PR will reinforce that belief.

I'd rather see this folded in than tracked — either have /events consult data_auth when events_auth is None (which matches what this PR did for /ledgers), or, if the two axes are deliberately independent, say so in docs/ai/mcp-server.md's hardening section and in the Behavior-changes note, so "filters the listing to the token's scope" doesn't read as a property of the deployment.

Happy to be told the axes are deliberately separate — but then the doc line is the fix, and I don't think it should ship silent either way.

Agreed it shouldn't ship silent. I went with the doc route plus a warning, not the data-auth fallback, and it's in 022af76.

Why not the fallback: query peers subscribe to /events, and with events auth off they connect without a token (--peer-events-token is optional). Their tokens also carry events claims and are checked against the events issuers, not the data ones. So falling back to data auth would lock existing peers out of any server that has data auth required.

What's in instead:

• The server logs a warning at startup when data auth is required and events auth is none, saying /events lists every ledger and its nameservice record to anyone. ServerConfig::events_open_under_data_auth has a unit test covering which combinations trigger it.
• configuration.md says in both the data-auth and events-auth sections that the two are independent. The MCP guide's hardening section tells you to set --events-auth-mode required and to give query peers a --peer-events-token.
• In the PR body, "filters the listing" is now scoped to /ledgers, and there's a paragraph on why /events stays separate.

🟡 MEDIUM-2 — get_data_model gets the scope check but not the policy layer

(in-diff range 301-312).

authorize() runs and then ledger_info(&req.ledger).execute() goes straight to the ledger with no wrap_policy_defaults and no identity. So on a ledger configured f:defaultAllow false, using the PR's own fixture:

sparql_query   (anonymous) -> rowCount 0        # the fix in this PR, working
get_data_model (anonymous) -> "## Dataset Statistics — Classes: 0, Total instances: 0,
                             Properties: 2, Triples (flakes): 2"

Your code didn't introduce this and the HTTP twin does the same thing — GET /v1/fluree/info/gov on that same locked ledger returns the full property list (http://example.org/name), the named-graph inventory including #config and #txn-meta, the commit id and the nameservice record. So get_data_model is consistent with the existing surface, which is a reasonable place to land.

What I'd change is the claim rather than the code. "Identity-less MCP queries now get the ledger's configured policy defaults, like every other anonymous read" reads as covering the endpoint, and tokenless_mcp_applies_the_ledgers_policy_defaults only exercises sparql_query. Narrowing that sentence to the query tool, and adding a get_data_model case to the test asserting the current behaviour, would pin down what's intended so the next person doesn't have to work it out from the diff.

(If you think schema-and-counts should in fact be policy-gated, that's a bigger call than this PR and touches /info too — that's the one thing here I'd genuinely leave to a separate decision rather than fold in.)

Did it the way you suggested (022af76):

• The PR body now says identity-less sparql_query calls get the policy defaults, and that get_data_model isn't policy-filtered, the same as GET /info.
• tokenless_mcp_applies_the_ledgers_policy_defaults now also calls get_data_model on the deny-by-default ledger and pins what it returns today (Triples (flakes): 2).
• Whether schema and counts should be policy-gated is filed as /info and MCP get_data_model describe a ledger whose policy defaults deny the caller #1981 (needs-decision), covering /info and get_data_model together. The test comment and the PR body point to it.

🟡 MEDIUM-3 — token_required makes MCP stricter than /query under Optional, which is right but undocumented

(in-diff range 263-293).

data_auth_mode != DataAuthMode::None puts Optional on the token-required side. So on a server running --data-auth-mode optional, an anonymous /query is served and an anonymous /mcp is a 401 — and the token that satisfies it then also needs ledger claims or it reaches nothing.

That's the safe direction and I'd have made the same call. But Optional exists precisely for the "accept tokens, don't require them" posture, and someone in that posture will hit an MCP 401 with no obvious cause. The docs/operations/configuration.md block added here covers none and "an MCP issuer is configured"; a row for optional would close it.

This is minor and non-blocking — but if you agree it's right, I'd rather see the line added now than lost in the backlog.

Added (022af76). configuration.md now says that under optional an anonymous /query is served and an anonymous /mcp call is a 401. The MCP guide's token table row for optional/required says the same.

⚪ NIT-4 — MCP accepts #fragment and @pin spellings the HTTP route rejects

(in-diff range 29-48).

authorize() runs the ledger through scope_id, which is LedgerRef::parse(raw)?.id — it strips urn:fluree:, a @pin and a #fragment before checking the scope. The tool then hands the raw string to the loader, which uses the stricter LedgerId::parse. So:

ledger "open@t:1" -> authorized, then "SPARQL query error: Invalid ledger id 'open@t:1':
                   ledger name cannot contain '@'"
ledger "open#g"   -> authorized, then "Query error: Unknown named graph '#g'"

versus POST /v1/fluree/query/gov%23config, which is a 500 at the route.

Nothing leaks — every closed* spelling I tried returned Ledger not found, so the gate is the conservative of the two parsers — and #1961 already tracks the db()/graph() half of this divergence. I mention it only because MCP is now a third parser pair in the same family, and it's the sort of thing that's cheap to make consistent while the area is open.

Made consistent (022af76). Each tool now parses ledger once with LedgerRef, checks scope on that parsed id, and loads the address rebuilt from it, so the gate and the loader can't disagree.

• A @ time suffix is refused before the scope check, with a message pointing at t.
• get_data_model refuses a # graph selector.
• urn:fluree: and #txn-meta still work for sparql_query, because the loader accepts them.

I didn't use the strict LedgerId::parse here, because it would have rejected those two spellings. Unit test: a_ledger_argument_is_parsed_once_for_scope_and_load.

⚪ NIT-5 — nice catch on the setting-groups.md correction

(in-diff range 17-25).

Not a finding — just want it on the record that changing "Policy … is switched on by the request, not by the ledger" to "by the ledger's configuration or by the request", and the f:defaultAllow row from "Only consulted for requests that carry policy inputs" to "Applies to requests with and without policy inputs", is the part of this PR most likely to save someone a bad afternoon. Documentation that described a security property incorrectly is worse than none, and this was that.

Thanks. That correction is the one piece of 160f098ed that survived the rebase: #1935 had already landed the same graph-builder fix on main, so that commit is now docs-only.