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

fleetd #778: scope TASK_READ to a caller's own ticket; gate push-loop nudges on Authz #785

Open
agent wants to merge 2 commits from worker/778-12988a-4 into main
pull from: worker/778-12988a-4
merge into: fleet:main
Conversation 0 Commits 2 Files Changed 10
+377 -83
agent commented 2026-10-05 20:09:57 +02:00
Member
Copy Link
Copy Source

Two small, independent changes, per the ticket's own scoping correction (narrower than the title).

Change 1 - Authz.permits's TASK_READ case was unconditionally closed to anyone but a primary, worker, or architect. A collaborator or an observer can create an async ticket via fleet_send(wait:false) but could never read it back. Added a callerOwnsTicket classifier (same pattern as the existing knownLeadOrCollaborator/knownObserverTarget classifiers), backed by a new MessageService.isTicketOwnedBy(ticket, callerOwner). Wired into both FleetMcp's fleet_poll handler and FleetApp's GET /tasks/{ticket}. Both call sites had a second bug that made this moot even with a correct policy: they were passing target/null into the authorization check instead of the actual ticket id, so ownership could never have been evaluated. Fixed. No change to DRAIN (fleet_poll{target}) or the REPLY/ASK own-pane rule, as instructed.

Change 2 - ReplyPushLoop nudged a pane to run a call Authz would refuse it (observed: an observer nudged 5x to fleet_poll(ticket=...)). Fixed generally, not as an observer special case: ReplyPushLoop now takes a (lead, ticket) -> boolean authorization predicate, consulted once at pendingTicketsFor (the one place every other lookup in the class reads from), so an unauthorized ticket is invisible to decide/injectNudge/bumpNudgeCounts alike. The real predicate (wired in FleetdAssembly) reconstructs the lead's Principal from its bare terminal via a new CallerResolver.resolveTerminal (extracted from the existing connection-based resolve()) and runs the same Authz.permits/isTicketOwnedBy check the real call would face. Reply and question nudges need no equivalent gate - SPAWN/SEND-to-a-worker are already primary/architect-only, so the lead resolved for those two sources always already has unconditional DRAIN/ANSWER. Also fixed the nudge-sent/failed log lines, which called any ticket creator a "lead" regardless of role.

Tests (both required pairs are positive+negative control):

  • AuthzTest: new TASK_READ ownership matrix for an observer and a collaborator, each with a negative control (ticket created by someone else); existing observer/collaborator denial tests' javadoc updated for accuracy (they use the default fail-closed classifier, not an absolute denial).
  • MessageServiceTest: isTicketOwnedBy direct test, same positive/negative pairing, plus an unknown-ticket case.
  • ReplyPushLoopTest: a forbidden ticket produces no nudge, paired with a positive control (same setup, authorizing predicate) proving the nudge still fires normally.

Build: mvn clean install from the worktree root - BUILD SUCCESS, Tests run: 2182, Failures: 0, Errors: 0, Skipped: 0 (includes PackageCyclesTest green; no new package-pair edge was needed since ReplyPushLoop's new dependency is a plain BiPredicate<String,String> built in FleetdAssembly, not a new import from msg into auth).

Out of scope, not touched, per the ticket: the already-correct REPLY/ASK ownsSession rule, and the separate "reply returned as unroutable" defect (ticket already resolved by the turn-completion scrape before the reply queue existed).

Two small, independent changes, per the ticket's own scoping correction (narrower than the title). **Change 1** - `Authz.permits`'s `TASK_READ` case was unconditionally closed to anyone but a primary, worker, or architect. A collaborator or an observer can create an async ticket via `fleet_send(wait:false)` but could never read it back. Added a `callerOwnsTicket` classifier (same pattern as the existing `knownLeadOrCollaborator`/`knownObserverTarget` classifiers), backed by a new `MessageService.isTicketOwnedBy(ticket, callerOwner)`. Wired into both `FleetMcp`'s `fleet_poll` handler and `FleetApp`'s `GET /tasks/{ticket}`. Both call sites had a second bug that made this moot even with a correct policy: they were passing `target`/`null` into the authorization check instead of the actual ticket id, so ownership could never have been evaluated. Fixed. No change to `DRAIN` (fleet_poll{target}) or the `REPLY`/`ASK` own-pane rule, as instructed. **Change 2** - `ReplyPushLoop` nudged a pane to run a call `Authz` would refuse it (observed: an observer nudged 5x to `fleet_poll(ticket=...)`). Fixed generally, not as an observer special case: `ReplyPushLoop` now takes a `(lead, ticket) -> boolean` authorization predicate, consulted once at `pendingTicketsFor` (the one place every other lookup in the class reads from), so an unauthorized ticket is invisible to `decide`/`injectNudge`/`bumpNudgeCounts` alike. The real predicate (wired in `FleetdAssembly`) reconstructs the lead's `Principal` from its bare terminal via a new `CallerResolver.resolveTerminal` (extracted from the existing connection-based `resolve()`) and runs the same `Authz.permits`/`isTicketOwnedBy` check the real call would face. Reply and question nudges need no equivalent gate - `SPAWN`/`SEND`-to-a-worker are already primary/architect-only, so the lead resolved for those two sources always already has unconditional `DRAIN`/`ANSWER`. Also fixed the nudge-sent/failed log lines, which called any ticket creator a "lead" regardless of role. **Tests** (both required pairs are positive+negative control): - `AuthzTest`: new TASK_READ ownership matrix for an observer and a collaborator, each with a negative control (ticket created by someone else); existing observer/collaborator denial tests' javadoc updated for accuracy (they use the default fail-closed classifier, not an absolute denial). - `MessageServiceTest`: `isTicketOwnedBy` direct test, same positive/negative pairing, plus an unknown-ticket case. - `ReplyPushLoopTest`: a forbidden ticket produces no nudge, paired with a positive control (same setup, authorizing predicate) proving the nudge still fires normally. **Build**: `mvn clean install` from the worktree root - BUILD SUCCESS, Tests run: 2182, Failures: 0, Errors: 0, Skipped: 0 (includes PackageCyclesTest green; no new package-pair edge was needed since ReplyPushLoop's new dependency is a plain `BiPredicate<String,String>` built in FleetdAssembly, not a new import from msg into auth). Out of scope, not touched, per the ticket: the already-correct `REPLY`/`ASK` ownsSession rule, and the separate "reply returned as unroutable" defect (ticket already resolved by the turn-completion scrape before the reply queue existed).
agent added 2 commits 2026-10-05 20:09:58 +02:00
fleetd #778: let a collaborator or observer read a ticket it created 3cbd84eb07 
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.
fleetd #778: never nudge a pane to run a call it may not perform
CI / shell-tests (pull_request) Failing after 11s
Details
CI / contract (pull_request) Successful in 58s
Details
CI / build (pull_request) Failing after 1m55s
Details
0dae97e6a9 
ReplyPushLoop nudged an observer pane to run fleet_poll(ticket=...) five
times, which Authz refused every time -- the pane was the right one to
nudge, but the instruction was one it could never follow. Gate ticket
nudges generally: ReplyPushLoop takes a (lead, ticket) -> boolean
authorization predicate, consulted at the one place every other lookup in
the class reads from (pendingTicketsFor), so a ticket the resolved lead may
not poll is invisible to decide/injectNudge/bumpNudgeCounts alike, never
named in a nudge, and never spends its own nudge budget.

The real predicate, wired in FleetdAssembly, reconstructs the resolved
lead's Principal from its bare terminal (CallerResolver.resolveTerminal,
extracted from the existing connection-based resolve()) and runs it through
the same Authz.permits/MessageService.isTicketOwnedBy check the real
fleet_poll call would face. Reply and question nudges need no equivalent
gate: Authz already restricts who can delegate to a worker in the first
place (SPAWN and SEND-to-a-worker are primary/architect-only), so the lead
resolved for those two sources is always one DRAIN/ANSWER already grants
unconditionally.

Also fixes the nudge-sent/failed log lines, which called any ticket creator
a "lead" regardless of its actual role.

Test: ReplyPushLoopTest proves a forbidden ticket produces no nudge, paired
with a positive control proving the same setup nudges normally once the
predicate authorizes the call.
Some checks are pending
CI / shell-tests (pull_request) Failing after 11s
Details
CI / contract (pull_request) Successful in 58s
Details
CI / build (pull_request) Failing after 1m55s
Details
This pull request has changes conflicting with the target branch.
  • fleetd/src/main/java/dev/ltms/fleet/auth/Authz.java
  • fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java
  • fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java
  • fleetd/src/test/java/dev/ltms/fleet/auth/AuthzTest.java
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin worker/778-12988a-4:worker/778-12988a-4
git checkout worker/778-12988a-4
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#785
Delete Branch "worker/778-12988a-4"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?