feat(server): accept a time pin in the /query ledger path - #1970

🟠 HIGH-1 — #fragment@pin slips past the guard added for exactly this ambiguity and silently reads HEAD

(in-diff range 1873-2125).

LedgerRef::parse splits on # before @, so gov:main#g@t:1 comes back as at: None, fragment: Some("g@t:1"). PathLedger::parse then takes the let Some(at) = parsed.at else { … pin: None } early return at :1899 and never reaches the fragment guard at :1906.

POST /v1/fluree/query/gov:main#g@t:1   -> 200 OK, 0 bindings   (HEAD, unknown named graph)
POST /v1/fluree/query/gov:main@t:1#g   -> 400 "combines a time pin with a graph fragment;
                                              select the graph in the query instead"

So one ordering gets the deliberate 400 and the transposed one gets the silent drop — which is the behaviour this PR argues throughout is worth a 400 for ("Otherwise the pin would be silently dropped"). A client that gets 200 OK and zero rows has no way to tell it was served HEAD rather than its snapshot.

It looks like a line in the else arm:

let Some(at) = parsed.at else {
  if parsed.fragment.as_deref().is_some_and(|f| f.contains('@')) {
      return Err(ServerError::bad_request(format!(
          "Ledger path '{raw}' combines a time pin with a graph fragment; \
           select the graph in the query instead"
      )));
  }
  return Ok(Self { ledger: raw.to_string(), id: parsed.id, pin: None });
};

One knock-on for the PR body: "A #fragment in an unpinned path … [is a] 500" isn't accurate for this shape — it's a 200. (I didn't separately test a bare mydb#g, so I can only say the claim doesn't hold for #g@t:1, not that it's wrong in general.)

Fixed in fb9ca13, as you suggested: PathLedger::parse now refuses a fragment containing @ with the same "combines a time pin with a graph fragment" 400 as @t:1#g. /stream/query goes through the same parser, so it gets the check too.

One thing changed underneath this after the rebase onto main: with #1958 merged, the transposed spelling already comes back as a 400 "Invalid ledger id … branch cannot contain '@'", because the loader's parse now maps to 400. So the fix now matters because it refuses the spelling at the path, with the right message, instead of relying on a later parse. Because of that, malformed_path_pin_is_a_400 checks for the pin-and-fragment message on /query and /explain, not just the status. It fails with the check reverted.

Thanks for the correction on the PR body. That line now describes what main does after the rebase.

🟡 MEDIUM-2 — a pinned read is governed by the policy as of the pin, and nothing says so

(in-diff range 100-109).

resolve_and_attach_config (fluree-db-api/src/view/fluree_ext.rs:136) resolves the ledger config from the pinned view's snapshot / overlay / t, so f:policyDefaults, f:defaultAllow and f:overrideControl are all read at the pin. A ledger that was open at t1 and locked down at t2 answers a @t:1 read with t1's permissive policy.

On a fixture where t1 is data and t2 adds <urn:cfg:policy> f:defaultAllow false:

This PR does not introduce the class — the last two rows are reachable on main today and this diff can't touch them (pin_jsonld_dataset returns early when pin is None, and the connection /query handler isn't in the diff). I don't think that makes it a blocker here.

What it does change is reach. A pin in the URL path is visible to things a JSON body isn't: WAF rules, proxy allowlists, audit-log greps, and the human judgement of "is this link safe to send someone". And there's no server switch to refuse pins at all — I looked for a time_travel / allow_time key in fluree-db-server/src/config.rs and there isn't one.

I'm honestly not sure the pin-consistent reading is wrong — a snapshot read arguably should be self-consistent, and policy here is model-governs-instance data that lives in the ledger. It's the silence I'd fix. One sentence in the new time-travel section — a pinned read is governed by the policy as it stood at the pin; locking a ledger down does not retroactively protect earlier states — closes the operator-surprise gap, and it's the kind of thing that's much cheaper to write now than to discover later.

Agreed the silence was the problem. docs/concepts/time-travel.md now has a short "Policy at a Past Point" section (fb9ca13). It says a read at a past point is governed by the ledger's policy defaults and rules as they stood at that point, however the time is given (a path pin, a body from, or FROM), so tightening policy later doesn't protect earlier states. It suggests limiting the reader's ledger scope instead, and links to the time-travel section of the policy docs, which already describes this behavior.

I wrote it as a description of current behavior. Whether a past read should use the policy as it stood then or as it stands now is a separate question I'd rather not settle in this PR.

🟡 MEDIUM-3 — PathLedger parses a canonical id and then hands the raw spelling to everything

(in-diff range 1873-2125).

PathLedger.id is the canonical LedgerId, but it's used only by pin_jsonld_source's names_path_ledger. The bearer scope check (:1162, :1191, :1255, :1483, :1570) and every loader (db_at, db_at_with_default_context, load_ledger_for_query, pinned_ledger_spec, ledger_scoped_sparql_dataset_spec) take path.ledger — the raw string.

The observable consequence:

POST /v1/fluree/query/urn:fluree:gov:main@t:1
-> 500 "Invalid ledger id 'urn:fluree:gov:main': branch cannot contain ':'"

The scope check accepted it as gov:main (scope_id is LedgerRef::parse(raw)?.id, which strips the urn); the loader's LedgerId::parse rejected it. Not exploitable — it fails closed, and only because the loader is the stricter of the two. But it's the one seam in this change where the id that was authorized and the id that gets read come from different parsers, and the unit test at :2568 currently pins the unloadable spelling as correct.

ledger: path.id.as_str().to_string() would close the seam and make urn:fluree: paths work, which retires one of the disclosed 500s for free. Worth noting #1961 already tracks the db()/graph() half of this same family — this would be a third parser pair in it, so there may be an argument for doing them together rather than piecemeal.

Agreed on the seam, but I'd rather do it with the #1961 family than here, because the one-liner isn't safe on its own. normalize_ledger_scoped_from and ledger_scoped_sparql_dataset_spec compare ledger strings as written (base_ledger_id strips the time and fragment but adds no default branch). If path.ledger became mydb:main, then /query/mydb with a body from: "mydb" or FROM <mydb>, which works today, would turn into a "Ledger mismatch".

The complete fix is to pass the canonical id downstream and switch those comparisons to typed ids. That also clears the pinned vs pinned:main mismatch listed in the body. It touches the whole route rather than the pin, so I'd like it in its own change.

Status since the rebase: a urn:fluree: path is now a 400 "Invalid ledger id", not a 500. It still fails closed.

⚪ NIT-4 — @t:latest forces the dataset lane although it reads HEAD

(in-diff range 3405-3413).

dataset_lane = has_dataset_clause || pin.is_some(), and TimeSpec::Latest is an accepted pin. So /query/mydb@t:latest — semantically identical to /query/mydb — skips the single-graph lane, the proxy-mode lane (:3428) and the connection-policy lane (:3437), and 406s on CSV/TSV. Collapsing Latest to pin = None in PathLedger::parse removes it.

This one is reasoned from the code rather than run, so treat it as a question rather than an established finding.

Leaving it as is. If @t:latest became "no pin", it would also skip the path/body conflict check, so a path saying @t:latest and a body saying @t:5 would stop being a "Time pin conflict". A body from: "mydb@t:latest" takes the dataset path too, so the two spellings stay consistent. The only cost is CSV/TSV returning 406 for a spelling that means head.

⚪ NIT-5 — a body ledger key 400s on a pinned path though the route never reads it

(in-diff range 1873-2125).

DATASET_KEYS includes "ledger", but get_ledger_id (:1281) returns the path ledger and never consults a body ledger on a ledger-scoped route. So /query/gov:main@t:1 with {"ledger":"other:main","select":…} is a 400 "the query's dataset does not read 'gov:main'", while the same body unpinned is accepted and the key ignored. Small inconsistency; dropping "ledger" from DATASET_KEYS on this route would align them.

I'd keep ledger in the list, because the engine does read it on the dataset path. DatasetSpec::from_query_json takes the default graph from opts.from || opts.ledger || query.from || query.ledger. A pinned request goes down the dataset path, and the pinned from we add sits at the top level, so an opts.ledger would outrank it.

If we dropped ledger from DATASET_KEYS, then /query/gov:main@t:1 with {"opts": {"ledger": "other:main"}, ...} would read other:main instead of being refused. The 400 is the safe answer there. The unpinned route ignores the key only because it takes the single-ledger path, which never looks at ledger.

⚪ NIT-6 — stream_query builds a whole PathLedger to read one boolean

(in-diff range 263-277).

Two allocations per streaming request, then dropped. LedgerRef::parse(&ledger)?.at.is_some() does the same with one. Genuinely trivial — mentioning it only because it's on a streaming path.

I'd keep PathLedger::parse here. Going through the same parser is what gives /stream/query the same refusals as /query, including the HIGH-1 check from this round. LedgerRef::parse(..)?.at.is_some() would miss #g@t:1, because that pin sits inside the fragment. The two small allocations seem worth it for not having a second parser on this route.