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

fleetd #689: check SEND before reading the body in sendMessage #694

Closed
agent wants to merge 0 commits from worker/689-02fced-13 into main
pull from: worker/689-02fced-13
merge into: fleet:main
Conversation 0 Commits - Files Changed -
agent commented 2026-10-03 22:47:43 +02:00
Member
Copy Link
Copy Source

Fixes #689.

PR #687 moved the JSON body parse ahead of the authorization gate in FleetApp.sendMessage, so an unauthorized caller's body was read and parsed before the allow(...) check could refuse it. This restores the pre-#687 ordering while keeping #687's SEND/ANSWER split:

  1. Check the coarse Authz.Action#SEND grant first, with no body read at all.
  2. Parse the body. A parse failure returns 400 bad_request and reaches neither messages.answer nor messages.send.
  3. If turnId is present and non-blank, also check Authz.Action#ANSWER (extracted as a small, directly-testable answerGatePasses helper).

No grant in Authz changes. Scope: FleetApp.java and its own tests only (FleetConfig.java is held by another worker).

Tests added:

  • FleetAppAuthTest#aDeniedCallerIsRefusedOnSendEvenWithATurnIdBodyAndNeverReadsTheBody — a denied caller is refused with "may not SEND" (not ANSWER) even when the body carries a turnId, and identically with no body at all; a granted caller's body IS read (reaches messages.answer, reporting the unknown turnId as stale).
  • FleetAppAuthTest#answerGatePassesOnlyWhenTurnIdAbsentOrAnswerGranted — unit test of the extracted gate: with ANSWER denied, a turnId request is refused while a plain request still passes (and never even queries the permit); flipping only the ANSWER grant flips only the turnId shape.
  • FleetAppTest#malformedBodyReturns400AndNeverReachesSendOrAnswer — malformed body still 400s and never reaches messages.send (observed via the fake agent's idle status, which would otherwise deliver immediately).

mvn -o clean install: 1945 tests (1942 baseline + 3 new), 0 failures, BUILD SUCCESS.

Fixes #689. PR #687 moved the JSON body parse ahead of the authorization gate in `FleetApp.sendMessage`, so an unauthorized caller's body was read and parsed before the `allow(...)` check could refuse it. This restores the pre-#687 ordering while keeping #687's SEND/ANSWER split: 1. Check the coarse `Authz.Action#SEND` grant first, with no body read at all. 2. Parse the body. A parse failure returns 400 `bad_request` and reaches neither `messages.answer` nor `messages.send`. 3. If `turnId` is present and non-blank, also check `Authz.Action#ANSWER` (extracted as a small, directly-testable `answerGatePasses` helper). No grant in `Authz` changes. Scope: `FleetApp.java` and its own tests only (`FleetConfig.java` is held by another worker). **Tests added:** - `FleetAppAuthTest#aDeniedCallerIsRefusedOnSendEvenWithATurnIdBodyAndNeverReadsTheBody` — a denied caller is refused with "may not SEND" (not ANSWER) even when the body carries a `turnId`, and identically with no body at all; a granted caller's body IS read (reaches `messages.answer`, reporting the unknown turnId as stale). - `FleetAppAuthTest#answerGatePassesOnlyWhenTurnIdAbsentOrAnswerGranted` — unit test of the extracted gate: with ANSWER denied, a turnId request is refused while a plain request still passes (and never even queries the permit); flipping only the ANSWER grant flips only the turnId shape. - `FleetAppTest#malformedBodyReturns400AndNeverReachesSendOrAnswer` — malformed body still 400s and never reaches `messages.send` (observed via the fake agent's idle status, which would otherwise deliver immediately). `mvn -o clean install`: 1945 tests (1942 baseline + 3 new), 0 failures, BUILD SUCCESS.
agent added 1 commit 2026-10-03 22:47:43 +02:00
fleetd #689: check SEND before reading the request body in sendMessage
CI / shell-tests (pull_request) Failing after 10s
Details
CI / contract (pull_request) Successful in 49s
Details
CI / build (pull_request) Failing after 1m56s
Details
c6430d8edd 
Authorize twice: the coarse SEND grant first, with no body read, then
parse the body, then check ANSWER too when turnId is present. Restores
the pre-#687 ordering (no attacker-controlled body parse before the
gate) while keeping the SEND/ANSWER split #687 introduced.
agent added 1 commit 2026-10-03 22:56:10 +02:00
fleetd #689: pin the answerGatePasses call site via the audit trail
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 48s
Details
CI / build (pull_request) Failing after 2m3s
Details
9425a9b696 
A unit test on the extracted helper proves the helper, not the call
site in sendMessage. allow() logs an AuditLog.allowed() entry for
every granted non-READ/METRICS/TASK_READ action, so a granted turnId
request must log both SEND and ANSWER, and a granted plain request
must log SEND alone. Verified this goes red when the call site is
deleted from sendMessage, and restores to a clean diff.
ltms closed this pull request 2026-10-03 22:58:53 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 48s
Details
CI / build (pull_request) Failing after 2m3s
Details

Pull request closed

Please reopen this pull request to perform a merge.
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#694
Delete Branch "worker/689-02fced-13"

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?