fleet/fleetd
3
0
Fork 0
Code Issues 105 Pull Requests 2 Actions Packages Projects Releases 1 Wiki Activity

fleetd #778: let a collaborator or observer read a ticket it created

Browse Source 
TASK_READ was unconditionally closed to anyone but a primary, worker, or
architect, so a non-worker peer that used fleet_send(wait:false) could never
collect its own async reply. Add a ticket-ownership classifier to
Authz.permits, following the same pattern as the existing SEND classifiers,
and expose MessageService.isTicketOwnedBy so FleetMcp's fleet_poll handler
and FleetApp's GET /tasks/{ticket} can build it. Both call sites also fix a
second bug: they were passing a blank/null authorization target instead of
the actual ticket id, so even a correct policy could never have been
evaluated against it.

Tests: AuthzTest gets a TASK_READ ownership matrix for an observer and a
collaborator, each paired with a negative control (a ticket created by
someone else). MessageServiceTest covers isTicketOwnedBy directly, same
pairing.
This commit is contained in:
Dai Ha
2026-10-05 20:09:24 +02:00
parent c043d149cf
commit 3cbd84eb07
6 changed files with 209 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
fleetd/src/main/java/dev/ltms/fleet/auth/Authz.java
+57 -19
View File
@@ -36,7 +36,12 @@ public final class Authz {
DRAIN,
/** Read-only roster, profile, and identity observation: no ticket, task, or turn state. */
READ,
/** Poll a ticket, or read a session's status. */
/**
* Poll a ticket, or read a session's status. Open unconditionally to a primary, a worker,
* and an architect; open to any other caller only for a ticket it created itself (see
* {@link #permits(Principal, Action, String, Predicate, Predicate, Predicate)}'s
* {@code callerOwnsTicket} parameter).
*/
TASK_READ,
/**
* Read (never ack) this daemon's own held lead-to-lead coordination mail (fleetd #421).
@@ -79,28 +84,52 @@ public final class Authz {
*/
public static final Predicate<String> NO_KNOWN_OBSERVER_TARGET = target -> false;
/**
* The fail-closed classifier for {@code TASK_READ}'s ticket-ownership grant: answers no for
* every ticket, so the grant is refused unless a caller supplies a real one backed by the
* ticket's recorded creator (see {@code MessageService#isTicketOwnedBy}).
*/
public static final Predicate<String> NO_CALLER_OWNS_TICKET = ticket -> false;
/**
* Convenience form for a caller with no classifier to supply. Fails closed: a collaborator's
* or an observer's {@code SEND} is refused, as if no terminal were a configured lead,
* collaborator, or observer target — the same decision {@link #NO_KNOWN_LEAD_OR_COLLABORATOR}
* and {@link #NO_KNOWN_OBSERVER_TARGET} give explicitly. Every other action's result is
* identical to the five-argument form's, since none of them consult either classifier.
* collaborator, or observer target, and {@code TASK_READ} for a ticket that caller did not
* create itself is refused too — the same decisions {@link #NO_KNOWN_LEAD_OR_COLLABORATOR},
* {@link #NO_KNOWN_OBSERVER_TARGET}, and {@link #NO_CALLER_OWNS_TICKET} give explicitly. Every
* other action's result is identical to the six-argument form's, since none of them consult
* any of the three classifiers.
*
* <p>Its default classifiers deny every collaborator and every observer, so a caller
* enforcing authorization must use the five-argument form instead.
* <p>Its default classifiers deny every collaborator, every observer, and every ticket a
* primary/worker/architect did not already have unconditionally, so a caller enforcing
* authorization must use the six-argument form instead.
*/
public static boolean permits(Principal caller, Action action, String targetSession) {
return permits(caller, action, targetSession, NO_KNOWN_LEAD_OR_COLLABORATOR, NO_KNOWN_OBSERVER_TARGET);
return permits(caller, action, targetSession, NO_KNOWN_LEAD_OR_COLLABORATOR,
NO_KNOWN_OBSERVER_TARGET, NO_CALLER_OWNS_TICKET);
}
/**
* As {@link #permits(Principal, Action, String)}, with a real classifier for a collaborator's
* {@code SEND}. An observer's {@code SEND} still fails closed ({@link #NO_KNOWN_OBSERVER_TARGET}) —
* a caller enforcing both grants must use the five-argument form.
* {@code SEND}. An observer's {@code SEND} and {@code TASK_READ}'s ticket-ownership grant
* still fail closed — a caller enforcing all three grants must use the six-argument form.
*/
public static boolean permits(Principal caller, Action action, String targetSession,
Predicate<String> knownLeadOrCollaborator) {
return permits(caller, action, targetSession, knownLeadOrCollaborator, NO_KNOWN_OBSERVER_TARGET);
return permits(caller, action, targetSession, knownLeadOrCollaborator,
NO_KNOWN_OBSERVER_TARGET, NO_CALLER_OWNS_TICKET);
}
/**
* As {@link #permits(Principal, Action, String, Predicate)}, with a real classifier for an
* observer's {@code SEND}. {@code TASK_READ}'s ticket-ownership grant still fails closed — a
* caller enforcing all three grants must use the six-argument form.
*/
public static boolean permits(Principal caller, Action action, String targetSession,
Predicate<String> knownLeadOrCollaborator,
Predicate<String> knownObserverTarget) {
return permits(caller, action, targetSession, knownLeadOrCollaborator, knownObserverTarget,
NO_CALLER_OWNS_TICKET);
}
/**
@@ -108,8 +137,9 @@ public final class Authz {
*
* @param targetSession the session id in the request path; only consulted for the
* worker-scoped actions ({@code REPLY}, {@code ASK}), for a
* collaborator's {@code SEND}, and for an observer's
* {@code SEND}, ignored otherwise, may be {@code null}
* collaborator's {@code SEND}, for an observer's {@code SEND},
* and (as the ticket id) for {@code TASK_READ}, ignored
* otherwise, may be {@code null}
* @param knownLeadOrCollaborator whether a terminal is a configured lead or collaborator —
* consulted only for a collaborator's {@code SEND}, to confine
* it to another named peer and never a spawned member's
@@ -118,10 +148,16 @@ public final class Authz {
* {@link Role#OBSERVER} — consulted only for an observer's
* {@code SEND}, to confine it to another observer pane and never
* a lead, a collaborator, or a spawned member
* @param callerOwnsTicket whether {@code targetSession} (read as a ticket id) was
* created by the calling session — consulted only for
* {@code TASK_READ} by a caller that is none of primary, worker,
* or architect, so a collaborator or an observer may read a
* ticket it created itself and no other
*/
public static boolean permits(Principal caller, Action action, String targetSession,
Predicate<String> knownLeadOrCollaborator,
Predicate<String> knownObserverTarget) {
Predicate<String> knownObserverTarget,
Predicate<String> callerOwnsTicket) {
if (caller == null || caller.isAnonymous()) {
return false; // authenticated as nothing ⇒ authorized for nothing
}
@@ -169,12 +205,14 @@ public final class Authz {
case READ, METRICS -> caller.isPrimary() || caller.isWorker() || caller.isArchitect()
|| caller.isCollaborator() || caller.isObserver();
// Ticket polling and session status, open to every role READ is open to except a
// collaborator or an observer. MessageService compares a ticket's creator to the
// caller on every read as well, so dropping this gate would not expose another
// session's reply — it would move the refusal later and widen what a caller that
// never orchestrates can probe.
case TASK_READ -> caller.isPrimary() || caller.isWorker() || caller.isArchitect();
// Ticket polling and session status, open unconditionally to every role READ is open
// to except a collaborator or an observer. A collaborator or an observer gets it too,
// but only for a ticket it created itself: fleet_send(wait:false) is open to both
// roles, so either can hold a ticket nobody else may read, and the ticket already
// records who created it. MessageService makes the same comparison again on every
// read, so this gate narrows WHO gets to ask, not what the answer is.
case TASK_READ -> caller.isPrimary() || caller.isWorker() || caller.isArchitect()
|| callerOwnsTicket.test(targetSession);
// fleetd #421: reading held lead-to-lead mail is the primary's alone. An architect
// holds READ today (CB-548), so "not primary" must mean not-architect here too — this
fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java
+36 -4
View File
@@ -563,11 +563,13 @@ public final class FleetMcp {
Map<String, Object> a = req.arguments();
String target = str(a, "target");
String coordId = str(a, "coordId");
String ticket = str(a, "ticket");
// The action depends on the ARGUMENTS, not on the tool name -- see pollAction.
McpSchema.CallToolResult denied = deny(exchange, toolAction("fleet_poll", a), target);
Authz.Action action = toolAction("fleet_poll", a);
McpSchema.CallToolResult denied = deny(exchange, action, pollAuthzTarget(action, ticket, target),
t -> t != null && messages.isTicketOwnedBy(t, principal(exchange).ownerKey()));
if (denied != null) return denied;
return poll(messages, leadChannel, str(a, "ticket"), target, coordId,
principal(exchange).ownerKey());
return poll(messages, leadChannel, ticket, target, coordId, principal(exchange).ownerKey());
};
// CB-307 Increment 3: per-msgId ack (not needed in v1 but supported by the inbox).
// Acking removes a reply from the inbox, so it is a drain, not a read.
@@ -732,6 +734,15 @@ public final class FleetMcp {
return denyFor(principal(exchange), action, target);
}
/**
* As {@link #deny(McpSyncServerExchange, Authz.Action, String)}, also threading the classifier
* {@code TASK_READ}'s ticket-ownership grant is checked against.
*/
private McpSchema.CallToolResult deny(McpSyncServerExchange exchange, Authz.Action action,
String target, Predicate<String> callerOwnsTicket) {
return denyFor(principal(exchange), action, target, callerOwnsTicket);
}
/**
* The policy half of {@link #deny}: everything except pulling the caller out of the MCP
* exchange. Kept separate so the authorization decision — the actual control — is unit-testable
@@ -745,13 +756,24 @@ public final class FleetMcp {
* @return {@code null} when the call may proceed, or the error result to return when it may not
*/
McpSchema.CallToolResult denyFor(Principal caller, Authz.Action action, String target) {
return denyFor(caller, action, target, Authz.NO_CALLER_OWNS_TICKET);
}
/**
* As {@link #denyFor(Principal, Authz.Action, String)}, also threading the classifier
* {@code TASK_READ}'s ticket-ownership grant is checked against.
*
* @return {@code null} when the call may proceed, or the error result to return when it may not
*/
McpSchema.CallToolResult denyFor(Principal caller, Authz.Action action, String target,
Predicate<String> callerOwnsTicket) {
// The enforcement switch lives HERE rather than in the exchange-facing wrapper: any future
// tool that calls this directly must not be able to skip the gate by accident.
if (!authorizationEnforced) {
return null; // AuthorizationMode.UNENFORCED: authorization not enforced (fleetd #518)
}
if (Authz.permits(caller, action, target, callers.knownLeadOrCollaborator(),
callers.sendableObserverTarget())) {
callers.sendableObserverTarget(), callerOwnsTicket)) {
if (action != Authz.Action.READ && action != Authz.Action.TASK_READ) {
AuditLog.allowed(caller, action, target); // reads would drown the trail
}
@@ -1215,6 +1237,16 @@ public final class FleetMcp {
return isBlank(target) ? Authz.Action.TASK_READ : Authz.Action.DRAIN;
}
/**
* The identifier {@code fleet_poll}'s authorization check runs against: the ticket for
* {@link Authz.Action#TASK_READ} (so a ticket-ownership classifier has something to test),
* {@code target} for every other action, exactly as {@link #pollAction} already decided.
* Pulled into its own method for the same testability reason as {@link #pollAction} itself.
*/
static String pollAuthzTarget(Authz.Action action, String ticket, String target) {
return action == Authz.Action.TASK_READ ? ticket : target;
}
/**
* Which authorization action a {@code fleet_send} call needs, decided by its arguments.
*
fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java
+11
View File
@@ -1400,6 +1400,17 @@ public final class MessageService {
return poll(ticket, INTERNAL_NO_OWNER_CHECK);
}
/**
* Whether {@code ticket} was created by the session identified by {@code callerOwner} — the
* same comparison {@link #poll(String, String)} itself makes, exposed so a caller can be
* granted read access to a ticket before it actually polls one. Returns {@code false} for an
* unknown or expired ticket.
*/
public boolean isTicketOwnedBy(String ticket, String callerOwner) {
Task task = tasks.get(ticket);
return task != null && Objects.equals(callerOwner, task.creatorOwner);
}
/**
* Snapshot the state of an async delegation. Returns {@code null} for an unknown/expired ticket.
* Refuses a {@code callerOwner} that differs from the owner that created the ticket (see
fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java
+27 -4
View File
@@ -294,6 +294,17 @@ public final class FleetApp {
return Authz.permits(caller, action, target, knownLeadOrCollaborator, knownObserverTarget);
}
/**
* As {@link #permitsFor(Principal, Authz.Action, String, Predicate, Predicate)}, also
* threading the classifier {@code TASK_READ}'s ticket-ownership grant is checked against.
*/
static boolean permitsFor(Principal caller, Authz.Action action, String target,
Predicate<String> knownLeadOrCollaborator,
Predicate<String> knownObserverTarget,
Predicate<String> callerOwnsTicket) {
return Authz.permits(caller, action, target, knownLeadOrCollaborator, knownObserverTarget, callerOwnsTicket);
}
/**
* Gate a handler on the CB-505 authorization table. Returns {@code true} when the request may
* proceed; otherwise writes the error response and returns {@code false}.
@@ -303,11 +314,20 @@ public final class FleetApp {
* yours" (a worker reaching for another worker's session, or for orchestration).
*/
private boolean allow(Context ctx, Authz.Action action, String target) {
return allow(ctx, action, target, Authz.NO_CALLER_OWNS_TICKET);
}
/**
* As {@link #allow(Context, Authz.Action, String)}, also threading the classifier
* {@code TASK_READ}'s ticket-ownership grant is checked against.
*/
private boolean allow(Context ctx, Authz.Action action, String target, Predicate<String> callerOwnsTicket) {
if (auth == null) {
return true; // legacy: authorization not enforced
}
Principal caller = ctx.attribute(CALLER);
if (permitsFor(caller, action, target, auth.knownLeadOrCollaborator(), auth.sendableObserverTarget())) {
if (permitsFor(caller, action, target, auth.knownLeadOrCollaborator(), auth.sendableObserverTarget(),
callerOwnsTicket)) {
if (action != Authz.Action.READ && action != Authz.Action.METRICS
&& action != Authz.Action.TASK_READ) {
AuditLog.allowed(caller, action, target); // reads would drown the trail
@@ -929,11 +949,14 @@ public final class FleetApp {
/** Poll an async (wait:false) delegation by ticket. 404 for an unknown/expired ticket. */
private void taskStatus(Context ctx) {
if (!allow(ctx, routeAction("GET /tasks/{ticket}"), null)) {
String ticket = ctx.pathParam("ticket");
Principal caller = ctx.attribute(CALLER);
String ownerKey = caller == null ? null : caller.ownerKey();
if (!allow(ctx, routeAction("GET /tasks/{ticket}"), ticket,
t -> t != null && messages.isTicketOwnedBy(t, ownerKey))) {
return;
}
Principal caller = ctx.attribute(CALLER);
MessageService.TaskView v = messages.poll(ctx.pathParam("ticket"), caller == null ? null : caller.ownerKey());
MessageService.TaskView v = messages.poll(ticket, ownerKey);
if (v == null) {
ctx.status(404).json(Map.of("error", "unknown_ticket", "detail", "no such task (or it has expired)"));
return;
fleetd/src/test/java/dev/ltms/fleet/auth/AuthzTest.java
+56 -7
View File
@@ -237,8 +237,11 @@ class AuthzTest {
}
/**
* Every action denied to a collaborator, asserted denied even when the classifier would
* accept any target — proving none of these is actually gated on the classifier at all.
* Every action denied to a collaborator, asserted denied even when the SEND classifier would
* accept any target — proving none of these is gated on that classifier. {@code TASK_READ} is
* included here with no ticket-ownership classifier supplied, so it falls back to the
* fail-closed default; its conditional grant for a ticket the collaborator actually created is
* tested separately below.
*/
@Test
void aCollaboratorIsDeniedLifecycleCoordinationAndTicketPolling() {
@@ -288,11 +291,12 @@ class AuthzTest {
}
/**
* Every action beyond READ/METRICS/REPLY/ASK/SEND, asserted denied for an observer —
* including {@code TASK_READ}, which is the entire point of this role: an unconfigured pane
* must not be able to poll a ticket or read another session's status. {@code SEND} is excluded
* here and given its own matrix below, since — unlike every action in this loop — its grant is
* conditional on the target, not fixed.
* Every action beyond READ/METRICS/REPLY/ASK/SEND, asserted denied for an observer with no
* ticket-ownership classifier supplied — so {@code TASK_READ} falls back to the fail-closed
* default here, an unconfigured pane reading a ticket it did not create. Its conditional grant
* for a ticket the observer actually created is tested separately below. {@code SEND} is
* excluded here and given its own matrix below, since — unlike every action in this loop — its
* grant is conditional on the target, not fixed.
*/
@Test
void anObserverIsDeniedEverythingBeyondReadMetricsReplyAskAndSend() {
@@ -363,4 +367,49 @@ class AuthzTest {
"an observer's SEND must consult the observer-target classifier, never the "
+ "lead-or-collaborator one");
}
// ── the TASK_READ ticket-ownership matrix ──────────────────────────────────────────────────────
/**
* {@code TASK_READ} for an observer is conditional on ticket ownership, exactly like
* {@code SEND} is conditional on the target: flipping only the ownership classifier's answer
* flips only this outcome. Paired with a negative control — the same observer, the same ticket,
* a classifier that reports a different creator — so a test that forgot to vary the classifier
* at all could not pass by accident.
*/
@Test
void anObserverMayTaskReadATicketOnlyWhenItCreatedIt() {
assertTrue(Authz.permits(OBSERVER, TASK_READ, "task-1", Authz.NO_KNOWN_LEAD_OR_COLLABORATOR,
Authz.NO_KNOWN_OBSERVER_TARGET, ticket -> true),
"a ticket the observer created must be readable");
assertFalse(Authz.permits(OBSERVER, TASK_READ, "task-1", Authz.NO_KNOWN_LEAD_OR_COLLABORATOR,
Authz.NO_KNOWN_OBSERVER_TARGET, ticket -> false),
"a ticket created by someone else must still be refused to the same observer");
assertFalse(Authz.permits(OBSERVER, TASK_READ, "task-1"),
"the default classifier owns no ticket, so the three-argument form still refuses");
}
/** As above, for a collaborator — the grant is the same mechanism, not an observer special case. */
@Test
void aCollaboratorMayTaskReadATicketOnlyWhenItCreatedIt() {
assertTrue(Authz.permits(COLLABORATOR, TASK_READ, "task-1", Authz.NO_KNOWN_LEAD_OR_COLLABORATOR,
Authz.NO_KNOWN_OBSERVER_TARGET, ticket -> true),
"a ticket the collaborator created must be readable");
assertFalse(Authz.permits(COLLABORATOR, TASK_READ, "task-1", Authz.NO_KNOWN_LEAD_OR_COLLABORATOR,
Authz.NO_KNOWN_OBSERVER_TARGET, ticket -> false),
"a ticket created by someone else must still be refused to the same collaborator");
}
/**
* Control: {@code TASK_READ} for a primary, a worker, and an architect does not move on the
* ticket-ownership classifier — it is already unconditional for all three.
*/
@Test
void theTicketOwnershipClassifierNeverGatesTaskReadForPrimaryWorkerOrArchitect() {
for (Principal p : new Principal[]{PRIMARY, WORKER_A, ARCH_DESIGN}) {
assertTrue(Authz.permits(p, TASK_READ, "task-1", Authz.NO_KNOWN_LEAD_OR_COLLABORATOR,
Authz.NO_KNOWN_OBSERVER_TARGET, Authz.NO_CALLER_OWNS_TICKET),
p.describe() + " must read a ticket unconditionally, even when the classifier owns nothing");
}
}
}
fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java
+22
View File
@@ -1034,6 +1034,28 @@ class MessageServiceTest {
assertEquals("forbidden: this ticket was created by a different session", refused.detail());
}
/**
* {@link MessageService#isTicketOwnedBy} is the primitive {@code Authz}'s {@code TASK_READ}
* grant is built on, exposed so a caller can be granted read access before it ever polls.
* Paired with a negative control: the same ticket, a different creator's owner key.
*/
@Test
void isTicketOwnedByMatchesOnlyTheRealCreator() {
Principal observerA = Principal.observer("term_observer_a", 1);
Principal observerB = Principal.observer("term_observer_b", 2);
String ticket = messages.sendAsync(T, "long task", null, observerA);
assertTrue(messages.isTicketOwnedBy(ticket, observerA.ownerKey()),
"the observer that created the ticket must be reported as its owner");
assertFalse(messages.isTicketOwnedBy(ticket, observerB.ownerKey()),
"a different observer must not be reported as the owner");
}
@Test
void isTicketOwnedByIsFalseForAnUnknownTicket() {
assertFalse(messages.isTicketOwnedBy("no-such-ticket", Principal.observer("term_x", 1).ownerKey()));
}
@Test
void architectOwnershipUsesTerminalRatherThanSlot() {
Principal oldArchitect = Principal.architect("opus", "term_OLD", 1);