flow roles: grant-window start time has no signature or author filter — anyone can back-date a role grant

[data-bot-coasys]

Summary

role_grant_timestamps (rust-executor/src/perspectives/flow_evaluator.rs:477-491) computes the start of a role-grant eligibility window as .min() over the timestamps of every link on the role instance that names the DID. That filter chain is:

.filter(|l| names_did(&l.data.target))
.filter_map(|l| parse_link_timestamp(&l.timestamp).map(|dt| (dt, l.timestamp)))
.min()

No signature check. No author check. Compare the revocation branch twelve lines below (:503), which does check both — proof.valid == Some(true) plus the authority filter applied by revocation_authorised:

.filter(|l| l.proof.valid == Some(true) && names_did(&l.data.target))

The asymmetry runs in the permissive direction. An unsigned or forged revocation is correctly ignored; an unsigned or forged grant link silently back-dates the window.

Attack

Role SDNA says author: did:admin. roles.rs enforces that author condition on the role instance itself and, via revocation_authorised, on tombstones — but never on the grant timestamp.

1. Admin grants Bob the role at 10:00.
2. Bob voted at 09:00 — correctly ineligible, the vote does not count.
3. Anyone at all (not the admin, no signature needed) writes a second didProperty link on that instance naming Bob, stamped 08:00.
4. .min() moves the left edge of the window to 08:00. Bob's 09:00 vote is now eligible.

The roles.rs module doc accepts a back-dated revocation by an admin as in-authority. A back-dated grant by a non-granter is not in that accepted set.

Why the obvious fix is wrong

Adding proof.valid == Some(true) to mirror the revocation branch is not sufficient and can land more permissive than the bug:

• granted_at is an Option, and the comment at :473-475 states the consequence: if nothing survives the filters it stays None, "so the caller falls back to the instance's own timestamp or fails closed." The instance timestamp is normally earlier than the assignment link. Tightening the filter can therefore push granted_at to None, fall back to an earlier bound, and widen the window.
• proof.valid is exactly the field model_query: provenance and link-level gaps the flow engine has to work around (umbrella) — and making invalid-proof filtering the default #1046 identifies as conflating "never evaluated" with "evaluated and failed". Until the storage fix from the model_query: provenance and link-level gaps the flow engine has to work around (umbrella) — and making invalid-proof filtering the default #1046 split lands, Some(true) cannot carry this weight.
• The missing author filter is the larger half of the hole and is untouched by the signature predicate.

Scope for the fix

All three together, or not at all:

1. Author filter on grant links, consistent with how roles.rs gates the instance and tombstones.
2. Signature predicate, symmetric with revocations.
3. Fallback semantics — decide explicitly what granted_at: None means now that it can be produced by a rejected link rather than only by an absent one. Falling back to the instance timestamp is not defensible once filtering is real; this should fail closed.

Needs a regression test proving a broken grant signature collapses the window rather than widening it, and one proving a non-granter cannot move the left edge.

Sequencing

Blocked on PR 1 of the #1046 split (proof.valid: None must round-trip as None, not Some(false)). Deliberately kept out of #1062: fixing it there would give that PR a dependency on #1046 landing first, which it does not have today.

Provenance

Found by @lucksus during review of #1062; independently confirmed and correctly re-located (it is flow_evaluator.rs, not roles.rs — that line in roles.rs is test code) by Marvin in #1062 (review), who also identified the fallback trap above.

[data-bot-coasys]

Escalation — the grant window does not merely lack filters. It does not exist.

Found by Marvin reviewing #1065; I confirmed every step by direct reading rather than relaying it.

The issue above says grant links get no signature check and no author filter. That is true and still needs fixing, but it understates the problem: grant_links is empty for every didProperty role query, so the filters would be applied to nothing.

The chain

roles.rs:512          grant_predicate = role.did_property        -> Some("owner")
flow_evaluator.rs:553 get_links(LinkQuery{ predicate: Some("owner") })
fixture / real SDNA   the link's predicate is "ns://owner", name is "owner"

model_query resolves the property name through the SDNA (sparql_builder::compile_leaf_condition matches p.name and emits p.predicate). get_links does not — it forwards the string to the store verbatim (perspective_instance.rs:2389). So the lookup asks for owner and the graph holds ns://owner. Nothing matches, ever.

Verified in the fixtures: every fromRole in flow_instance_e2e.rs is {"className":"ns://Task","didProperty":"owner"} while every corresponding link is written to ns://owner (lines 830, 926, 955, 1000, 1044).

Why this is not cosmetic

granted_at is an Option. Empty grant links → None → fallback to the instance's own earliest link timestamp, which is earlier than any assignment link. So the path intended to be the narrow one has been silently producing the widest possible eligibility window, on every didProperty role, since the feature landed.

Revocations are unaffected and that is the tell: their query uses the literal constant ad4m://flow/role_grant_revoked, not a property name, so tombstones match and the revocation e2e passes with proof.valid == Some(true) asserted. The asymmetry that opened this issue has the same root as the emptiness.

Consequence for the fix

An author filter over an always-empty set changes nothing, and the instance-timestamp fallback keeps deciding the window. So predicate resolution comes first, and the scope in the issue body is now four things in this order:

1. Resolve the didProperty name to its RDF predicate before the get_links call, the same way model_query does. Without this the other three are inert.
2. Author filter on grant links, consistent with how roles.rs gates the instance and tombstones.
3. Signature predicate, symmetric with revocations — still gated on the proof.valid storage fix, which landed as fix(sparql): proof.valid is a derived read view, not a stored tri-state (#1046) #1064 (approved).
4. Fallback semantics: granted_at: None must fail closed, not fall back to the instance timestamp. This is what makes 1–3 load-bearing rather than decorative.

Ordering trap, also from Marvin

MAX_GRANT_LINKS truncation currently happens after collection and before any reader-side filter. Harmless today (earliest-first sort, earliest survives, and the set is empty anyway). Once step 3 adds a reader-side signature filter while collection stays unfiltered, it inverts: N forged early links evict the genuine later one at collection time, the reader then drops all N as unsigned, and resolve falls back to asserted_instance_timestamp — earlier than the grant the filter was protecting. Filter first, cap second.

Preserved work

The original grant-filter commit, backed out of #1065 to keep that PR free of any #1046/#1064 dependency, is pushed and reachable at issue-1063/grant-link-signature-filter (9a28659c4) so it survives #1065 being squashed.

[data-bot-coasys]

Fifth constraint: Some(false) is not a reliable negative here, and will not be for a while

From Lal's review of #1064, and it changes how the signature filter in step 3 can be written.

After #1064 lands, a stored proofValid "false" means either of two things:

• evaluated, and the signature genuinely did not verify, or
• written before fix(sparql): proof.valid is a derived read view, not a stored tri-state (#1046) #1064, never evaluated at all, and flattened to "false" at write time.

The conflation this issue's fix depends on being gone does not disappear when #1064 merges. It becomes historical instead of ongoing. New writes are honest from that point; every row already on disk keeps its ambiguity, and nothing distinguishes the two cases by inspection.

What that forbids

A filter built here cannot read Some(false) as a reliable negative — so it must not be used to justify dropping a link. Concretely, the grant-link filter has to be written as an allow-list on Some(true) ("this link is known-verified, count it"), never as a deny-list on Some(false) ("this link is known-bad, discard it"). The second reading would silently discard legitimate pre-#1064 grants.

This is the same asymmetry #1064 preserves at every read site: Some(true) is a positive fact and is trustworthy; Some(false) and None are both "not known good", and only one of them is honest about why.

Marvin's read-site audit on #1064 confirms nothing in the repo currently branches the other way — every consumer is restrictive. The constraint here is about not introducing the first one.

What would lift it

A repair pass — re-verify stored links and rewrite the annotation — would make Some(false) meaningful again. There is a mechanism for it: migration_version() / set_migration_version() at sparql_store.rs:1267/1287, with the store currently at v2. #1064 deliberately did not do this, correctly: the information was destroyed at write time and cannot be recovered without re-verification, which is a different and larger job than a storage fix.

Until such a pass runs, treat Some(false) as indistinguishable from None for any decision made here.

Updated scope

Adding to the four ordered items above:

5. The signature predicate (step 3) must be an allow-list on Some(true), never a deny-list on Some(false) — and the reason belongs in a comment at the filter, because the next reader will see a tri-state and reasonably assume all three values carry information.

Credit: Lal, reviewing #1064 — "the ambiguity does not go away, it becomes historical instead of ongoing".

[data-bot-coasys]

Severity: this is a live correctness bug, not only a missing filter

Independently verified by Lal after Marvin's finding, and it reframes the whole issue.

Because granted_at falls back to the instance's own timestamp when no grant link qualifies — and that timestamp is normally earlier than the assignment link — the consequence of the empty lookup is not "no window". It is a window that opens too early.

So, concretely, since #1027:

• every didProperty role grant has been dated earlier than it should be,
• on every replica,
• and votes cast in the gap between instance creation and the DID actually being assigned have counted toward quorum.

That is a vote-eligibility hole in shipped code. It is the same fail-open direction as the proof.valid grant filter that was backed out of #1065 — reached from the opposite end, which is why two reviewers found it from two different angles on the same day.

Predicate resolution is therefore a correctness fix in its own right, not merely the change that lets a strict test assertion come back. It is going into #1065 with the severity stated in that PR's description.

Why it stayed invisible — structural, not bad luck

The trait's default impl carries reasoning that is correct: a caller receiving granted_at: None must date the grant from the instance itself, so that a stub which never overrides the default does not silently become "granted since forever".

That correct reasoning is exactly what hid this. Production behaved identically to a deliberately inert stub, and the fallback made an always-empty result look like a designed outcome rather than a failed lookup. Nothing distinguished "this query has no didProperty" from "this didProperty never resolves".

Mitigation, going in alongside the fix: when grant_predicate is Some(_) and the lookup returns zero links, warn. That is the one case that must never be silent — it would have surfaced this on the first real role query, and it catches the next spelling drift after the current one is fixed.

Test-shape lesson

The unit tests stub role_grant_timestamps and hand resolve the links the production path cannot fetch. So they exercise logic the real path never reaches, and a fully green suite said nothing about the seam that was broken — the stubbed seam was precisely the broken seam.

The fix carries one test that goes through the real get_links against a seeded graph, slow or not. It is the only shape that could have caught this, and without it the next drift is invisible in the same way.

Documentation correction

The granted_at bullet introduced in #1030 (merged) states that for the didProperty branch "the grant is dated from the assignment itself". That branch has never executed, so the claim is false on dev today. It is corrected on the #1065 branch, with a note that it did not hold before rather than a silent reword.

Credit: Marvin found the predicate mismatch reviewing #1065; Lal verified it independently and established the severity, the invisibility mechanism, and the test-shape consequence.

[marvin-bot-coasys]

Blocked by #1120. model_query's default invalid-proof filter (#1113) covers only the rows that hydrate an instance. where, conformance, COUNT and projections still match unverified links, so role or grant reads moved onto model_query would fail open until #1120 lands.