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

FleetApp.sendMessage parses the request body before the authorization gate #689

New Issue
Closed
opened 2026-10-03 22:27:09 +02:00 by ltms · 3 comments
ltms commented 2026-10-03 22:27:09 +02:00
Owner
Copy Link
Copy Source

Found while reviewing PR #687 (#669 Unit A). Filed rather than held, because the authorization
split in #687 is correct on its own and I merged it. This is the one thing in that PR I did not
want to lose.

What changed

PR #687 moved the JSON body parse in FleetApp.sendMessage to before the allow(...) call.
On origin/main the order was the other way round.

origin/main:

String id = ctx.pathParam("id");
if (!allow(ctx, routeAction("POST /sessions/{id}/message"), id)) {
    return;
}
...
JsonNode body = mapper.readTree(ctx.body());

After #687:

String id = ctx.pathParam("id");
JsonNode body;
try {
    body = mapper.readTree(ctx.body());
} catch (Exception e) {
    body = null;
}
String turnId = body == null ? null : body.path("turnId").asText(null);
if (!allow(ctx, routeAction("POST /sessions/{id}/message", turnId), id)) {
    return;
}

The reason was sound: turnId decides whether the call is a SEND or an ANSWER, and the
authorization check now takes that action. But the effect is that an unauthorized caller's request
body is read and parsed into a Jackson tree before the gate refuses it.

Why it does not need to be this way

I checked Authz on the PR branch. All three send actions carry the same grant:

case SEND       -> caller.isPrimary() || caller.isArchitect();
case ANSWER     -> caller.isPrimary() || caller.isArchitect();
case COORD_SEND -> caller.isPrimary() || caller.isArchitect();

A reviewer confirmed this across the whole table: 156 (role, action, target) pairs compared against
origin/main, 0 mismatches. So today turnId changes nothing about the decision, and the
reordering buys nothing. It is only needed if these grants ever diverge.

What I did not measure

I did not measure how large a body Jetty or Javalin accepts here. maxRequestSize is not
configured anywhere in this repo, so Javalin 6.7.0's default applies, and I did not look up what
that default is. A reviewer reported it as 1,000,000 bytes; I am repeating that as its claim, not
as a number I checked.

I also did not drive this with a real HTTP request. The reviewer I assigned read the code but did
not run anything, because it understood the reviewer contract to forbid running tests. That is a
defect in my brief, not in its work. So the DoS reading below is not confirmed by measurement —
what is confirmed is the ordering change, which I read in both versions of the file.

Practical risk

Low, in my judgement. bind.host is 127.0.0.1, so the caller must already be a local process,
and a local process can do worse things than make the daemon parse some JSON. I merged #687 on that
basis. The reason to fix it anyway is the principle: do not do attacker-controlled work before the
authorization gate, because the grants are meant to diverge later and the ordering will then matter.

Suggested fix

Authorize twice, cheaply. Check the coarse SEND grant first, with no body read. Then parse. Then,
if turnId is present, check ANSWER as well. That restores origin/main's ordering and still
enforces the finer action, and it keeps working if the two grants diverge.

Setting a route-appropriate maxRequestSize would be worth doing regardless, but it is a separate
change.

Also worth checking in the same pass

Two pre-existing things, neither introduced by #687, both reported by workers and both left alone
on purpose:

  • FleetApp.allow() leaves out READ and METRICS from the audit-log "allowed" trail, while
    FleetMcp.denyFor() leaves out only READ. #687 preserved that asymmetry and added TASK_READ
    to each list in its existing style. If one of the two is wrong, it was wrong before #687.
  • The malformed-body fallback picks SEND for the authorization check and then returns 400. I
    traced it and it reaches neither messages.answer nor messages.send, so nothing acts on the
    unparsed body. That is fine today, but it is a second place that stops being obviously fine if
    the grants diverge.
Found while reviewing PR #687 (#669 Unit A). Filed rather than held, because the authorization split in #687 is correct on its own and I merged it. This is the one thing in that PR I did not want to lose. ## What changed PR #687 moved the JSON body parse in `FleetApp.sendMessage` to **before** the `allow(...)` call. On `origin/main` the order was the other way round. `origin/main`: ```java String id = ctx.pathParam("id"); if (!allow(ctx, routeAction("POST /sessions/{id}/message"), id)) { return; } ... JsonNode body = mapper.readTree(ctx.body()); ``` After #687: ```java String id = ctx.pathParam("id"); JsonNode body; try { body = mapper.readTree(ctx.body()); } catch (Exception e) { body = null; } String turnId = body == null ? null : body.path("turnId").asText(null); if (!allow(ctx, routeAction("POST /sessions/{id}/message", turnId), id)) { return; } ``` The reason was sound: `turnId` decides whether the call is a `SEND` or an `ANSWER`, and the authorization check now takes that action. But the effect is that an unauthorized caller's request body is read and parsed into a Jackson tree before the gate refuses it. ## Why it does not need to be this way I checked `Authz` on the PR branch. All three send actions carry the same grant: ``` case SEND -> caller.isPrimary() || caller.isArchitect(); case ANSWER -> caller.isPrimary() || caller.isArchitect(); case COORD_SEND -> caller.isPrimary() || caller.isArchitect(); ``` A reviewer confirmed this across the whole table: 156 (role, action, target) pairs compared against `origin/main`, 0 mismatches. So **today** `turnId` changes nothing about the decision, and the reordering buys nothing. It is only needed if these grants ever diverge. ## What I did not measure I did not measure how large a body Jetty or Javalin accepts here. `maxRequestSize` is not configured anywhere in this repo, so Javalin 6.7.0's default applies, and I did not look up what that default is. A reviewer reported it as 1,000,000 bytes; I am repeating that as its claim, not as a number I checked. I also did not drive this with a real HTTP request. The reviewer I assigned read the code but did not run anything, because it understood the reviewer contract to forbid running tests. That is a defect in my brief, not in its work. So the DoS reading below is **not confirmed by measurement** — what is confirmed is the ordering change, which I read in both versions of the file. ## Practical risk Low, in my judgement. `bind.host` is `127.0.0.1`, so the caller must already be a local process, and a local process can do worse things than make the daemon parse some JSON. I merged #687 on that basis. The reason to fix it anyway is the principle: do not do attacker-controlled work before the authorization gate, because the grants are meant to diverge later and the ordering will then matter. ## Suggested fix Authorize twice, cheaply. Check the coarse `SEND` grant first, with no body read. Then parse. Then, if `turnId` is present, check `ANSWER` as well. That restores `origin/main`'s ordering and still enforces the finer action, and it keeps working if the two grants diverge. Setting a route-appropriate `maxRequestSize` would be worth doing regardless, but it is a separate change. ## Also worth checking in the same pass Two pre-existing things, neither introduced by #687, both reported by workers and both left alone on purpose: - `FleetApp.allow()` leaves out `READ` and `METRICS` from the audit-log "allowed" trail, while `FleetMcp.denyFor()` leaves out only `READ`. #687 preserved that asymmetry and added `TASK_READ` to each list in its existing style. If one of the two is wrong, it was wrong before #687. - The malformed-body fallback picks `SEND` for the authorization check and then returns 400. I traced it and it reaches neither `messages.answer` nor `messages.send`, so nothing acts on the unparsed body. That is fine today, but it is a second place that stops being obviously fine if the grants diverge.
ltms commented 2026-10-03 22:36:21 +02:00
Author
Owner
Copy Link
Copy Source

Delegated, with the fix approach fixed by me

Worker term_65cf596dc279d50 on branch worker/689-02fced-13, ticket task-13.

I re-read the code myself before briefing, rather than trusting this ticket's own quote. On main at 2eb2d61 the defect is live exactly as filed, at FleetApp.java:633-642: mapper.readTree(ctx.body()) runs, then turnId is lifted, then allow(...). The route's action split is at FleetApp.java:69, where a blank or null turnId maps to SEND and a present one to ANSWER.

Note the path in the original report is wrong in one detail: the file is fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java, not .../api/FleetApp.java. Nothing else in the report changed on re-reading.

The approach is this ticket's own suggested fix, and it is my decision, not the worker's: check the coarse SEND grant first with no body read, then parse, then check ANSWER as well when turnId is present. No grant in Authz changes — all three send actions carry the same grant today and this must not alter who can call what.

Acceptance, as briefed. Three properties, each with a required control:

  1. A caller refused the SEND grant gets no body parse. Control: the same caller, granted, reaches the body.
  2. With SEND allowed and ANSWER denied, a turnId request is refused while a plain one succeeds. Flipping only the ANSWER arm must change only the turnId shape. That control is what proves two separate checks rather than one renamed check.
  3. A malformed body still returns 400 and still reaches neither messages.answer nor messages.send.

The worker was told to say plainly if it could not drive one of these as a unit test, instead of reporting an unexercised property as held.

Scope fence. FleetApp.java and its own tests only. FleetConfig.java is held by another worker right now (#677), so I excluded it explicitly to avoid a collision in shared test files.

Still not measured, and not in this unit

The DoS reading remains unmeasured. I did not drive this with an HTTP request either, and I did not look up Javalin 6.7.0's default maxRequestSize. The reviewer's 1,000,000-byte figure is still that reviewer's claim, repeated, not a number anyone here has checked. Setting a route-appropriate maxRequestSize stays a separate change and is not in this unit.

The two pre-existing items in the "also worth checking" section above are not in this unit either: the allow()/denyFor() audit-log asymmetry, and the malformed-body fallback picking SEND. Both predate #687 and neither blocks the ordering fix.

Why this is being done now

#669 Unit B gives COLLABORATOR a different send grant, and at that moment this stops being cosmetic. Unit B also touches FleetConfig.java's validators, which the #677 worker holds, so Unit B is not delegated yet. This fix lands first, on its own, which is the order #669 asked for.

## Delegated, with the fix approach fixed by me Worker `term_65cf596dc279d50` on branch `worker/689-02fced-13`, ticket `task-13`. **I re-read the code myself before briefing**, rather than trusting this ticket's own quote. On `main` at `2eb2d61` the defect is live exactly as filed, at `FleetApp.java:633-642`: `mapper.readTree(ctx.body())` runs, then `turnId` is lifted, then `allow(...)`. The route's action split is at `FleetApp.java:69`, where a blank or null `turnId` maps to `SEND` and a present one to `ANSWER`. Note the path in the original report is wrong in one detail: the file is `fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java`, not `.../api/FleetApp.java`. Nothing else in the report changed on re-reading. **The approach is this ticket's own suggested fix, and it is my decision, not the worker's**: check the coarse `SEND` grant first with no body read, then parse, then check `ANSWER` as well when `turnId` is present. No grant in `Authz` changes — all three send actions carry the same grant today and this must not alter who can call what. **Acceptance, as briefed.** Three properties, each with a required control: 1. A caller refused the `SEND` grant gets no body parse. Control: the same caller, granted, reaches the body. 2. With `SEND` allowed and `ANSWER` denied, a `turnId` request is refused while a plain one succeeds. Flipping only the `ANSWER` arm must change only the `turnId` shape. That control is what proves two separate checks rather than one renamed check. 3. A malformed body still returns 400 and still reaches neither `messages.answer` nor `messages.send`. The worker was told to say plainly if it could not drive one of these as a unit test, instead of reporting an unexercised property as held. **Scope fence.** `FleetApp.java` and its own tests only. `FleetConfig.java` is held by another worker right now (#677), so I excluded it explicitly to avoid a collision in shared test files. ## Still not measured, and not in this unit The DoS reading remains unmeasured. I did not drive this with an HTTP request either, and I did not look up Javalin 6.7.0's default `maxRequestSize`. The reviewer's 1,000,000-byte figure is still that reviewer's claim, repeated, not a number anyone here has checked. Setting a route-appropriate `maxRequestSize` stays a separate change and is not in this unit. The two pre-existing items in the "also worth checking" section above are **not** in this unit either: the `allow()`/`denyFor()` audit-log asymmetry, and the malformed-body fallback picking `SEND`. Both predate #687 and neither blocks the ordering fix. ## Why this is being done now #669 Unit B gives `COLLABORATOR` a different send grant, and at that moment this stops being cosmetic. Unit B also touches `FleetConfig.java`'s validators, which the #677 worker holds, so Unit B is not delegated yet. This fix lands first, on its own, which is the order #669 asked for.
ltms commented 2026-10-03 22:51:41 +02:00
Author
Owner
Copy Link
Copy Source

Correction for the worker on task-13. Read this before you continue. It overrides the brief.

PR #694's fix is correct and I am keeping it. Build verified by me, trial-merged onto origin/main at bfee23a in a throwaway worktree, rm -rf target/surefire-reports, mvn -o clean install: 1948 tests, 0 failures, BUILD SUCCESS, exit=0, 173 report files. (1945 is the current baseline, not 1942 — main moved when #691 landed. 1945 + 3 = 1948.)

One thing needs adding before I merge.

The answerGatePasses call site is not pinned, only the helper is

I deleted the call site from sendMessage:

        if (!answerGatePasses(turnId, action -> allow(ctx, action, id))) {
            return;
        }

and ran mvn -o -Dtest='FleetAppAuthTest,FleetAppTest' test:

Tests run: 56, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

All 56 passed with the ANSWER gate removed entirely. So the suite cannot tell whether sendMessage still calls it. A unit test on the extracted helper proves the helper; it says nothing about the caller. That matters here more than usual, because the gate is behaviourally invisible today — the one thing that could notice it disappearing is a test, and no test does.

That is the exact risk this ticket was filed to close. Between now and #669 Unit B, a refactor could drop that call and every build stays green; at the moment Unit B gives COLLABORATOR a different grant, the ANSWER gate is silently gone.

Your "impossible today" was right about grants and too strong overall

You were correct that no real caller can produce "SEND allowed, ANSWER denied": I read it too, Authz.permits is a static call from FleetApp.allow(), so there is no seam to substitute and the scenario is unreachable without editing Authz. Extracting the helper to unit-test that case was a reasonable call and I am not asking you to undo it.

But the call site is observable by another route you did not consider. allow() writes to the audit trail on the allowed path:

if (Authz.permits(caller, action, target)) {
    if (action != Authz.Action.READ && action != Authz.Action.METRICS
            && action != Authz.Action.TASK_READ) {
        AuditLog.allowed(caller, action, target);
    }

ANSWER is none of those three, so a granted primary posting a body with a turnId must produce two allowed entries — one SEND, one ANSWER. With the call site deleted, only SEND appears.

The project already has the instrument: fleetd/src/test/java/dev/ltms/fleet/testing/CapturedLog.java, and AuditLogTest already observes the audit logger.

What to add

One test: a granted primary posts a body carrying a turnId, and the captured audit trail contains an allowed entry for both SEND and ANSWER.

Two controls, and report both:

  1. It must go RED when the call site is deleted. Delete those four lines, run it, paste the real failure. Restore, and show git diff on FleetApp.java is empty. This is the control that matters — it is the one my check above failed.
  2. A plain request with no turnId must produce a SEND entry and no ANSWER entry. That stops the new test passing just because something somewhere logs ANSWER.

Keep every test you already wrote. Properties 1 and 3 are genuinely pinned end-to-end and I verified the reasoning behind property 1 — a denied caller with a turnId body refused on SEND rather than ANSWER really does prove the ordering, because the 403 detail string interpolates the action name.

Unchanged

Still FleetApp.java and its own tests only. Still no change to Authz or any grant. Still no git add -A. Same build procedure, and the baseline is now 1945.

Push to the same branch and the same PR; do not open a second one. Reply with the new test's name, both controls as raw output, and the fresh build tail.

## Correction for the worker on `task-13`. Read this before you continue. It overrides the brief. PR #694's fix is **correct** and I am keeping it. Build verified by me, trial-merged onto `origin/main` at `bfee23a` in a throwaway worktree, `rm -rf target/surefire-reports`, `mvn -o clean install`: **1948 tests, 0 failures, BUILD SUCCESS**, `exit=0`, 173 report files. (1945 is the current baseline, not 1942 — `main` moved when #691 landed. 1945 + 3 = 1948.) One thing needs adding before I merge. ### The `answerGatePasses` call site is not pinned, only the helper is I deleted the call site from `sendMessage`: ```java if (!answerGatePasses(turnId, action -> allow(ctx, action, id))) { return; } ``` and ran `mvn -o -Dtest='FleetAppAuthTest,FleetAppTest' test`: ``` Tests run: 56, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` **All 56 passed with the ANSWER gate removed entirely.** So the suite cannot tell whether `sendMessage` still calls it. A unit test on the extracted helper proves the helper; it says nothing about the caller. That matters here more than usual, because the gate is behaviourally invisible today — the one thing that could notice it disappearing is a test, and no test does. That is the exact risk this ticket was filed to close. Between now and #669 Unit B, a refactor could drop that call and every build stays green; at the moment Unit B gives `COLLABORATOR` a different grant, the ANSWER gate is silently gone. ### Your "impossible today" was right about grants and too strong overall You were correct that no real caller can produce "SEND allowed, ANSWER denied": I read it too, `Authz.permits` is a `static` call from `FleetApp.allow()`, so there is no seam to substitute and the scenario is unreachable without editing `Authz`. Extracting the helper to unit-test that case was a reasonable call and I am not asking you to undo it. But the call site is observable by another route you did not consider. `allow()` writes to the audit trail on the **allowed** path: ```java if (Authz.permits(caller, action, target)) { if (action != Authz.Action.READ && action != Authz.Action.METRICS && action != Authz.Action.TASK_READ) { AuditLog.allowed(caller, action, target); } ``` `ANSWER` is none of those three, so a granted primary posting a body **with** a `turnId` must produce **two** allowed entries — one `SEND`, one `ANSWER`. With the call site deleted, only `SEND` appears. The project already has the instrument: `fleetd/src/test/java/dev/ltms/fleet/testing/CapturedLog.java`, and `AuditLogTest` already observes the `audit` logger. ### What to add One test: a granted primary posts a body carrying a `turnId`, and the captured audit trail contains an allowed entry for **both** `SEND` and `ANSWER`. Two controls, and report both: 1. **It must go RED when the call site is deleted.** Delete those four lines, run it, paste the real failure. Restore, and show `git diff` on `FleetApp.java` is empty. This is the control that matters — it is the one my check above failed. 2. **A plain request with no `turnId` must produce a `SEND` entry and no `ANSWER` entry.** That stops the new test passing just because something somewhere logs `ANSWER`. Keep every test you already wrote. Properties 1 and 3 are genuinely pinned end-to-end and I verified the reasoning behind property 1 — a denied caller with a `turnId` body refused on `SEND` rather than `ANSWER` really does prove the ordering, because the 403 detail string interpolates the action name. ### Unchanged Still `FleetApp.java` and its own tests only. Still no change to `Authz` or any grant. Still no `git add -A`. Same build procedure, and the baseline is now **1945**. Push to the same branch and the same PR; do not open a second one. Reply with the new test's name, both controls as raw output, and the fresh build tail.
ltms commented 2026-10-03 22:58:46 +02:00
Author
Owner
Copy Link
Copy Source

Fixed and merged. Closing.

PR #694 merged to main as edbd8d8, pushed (d0688c8..edbd8d8).

The shipped order in FleetApp.sendMessage is now:

    private void sendMessage(Context ctx) {
        String id = ctx.pathParam("id");
        if (!allow(ctx, routeAction("POST /sessions/{id}/message"), id)) {
            return;
        }
        JsonNode body;
        try {
            body = mapper.readTree(ctx.body());
        } catch (Exception e) {
            body = null;
        }
        if (body == null) {
            ctx.status(400).json(Map.of("error", "bad_request", "detail", "body must be JSON"));
            return;
        }
        String turnId = body.path("turnId").asText(null);
        if (!answerGatePasses(turnId, action -> allow(ctx, action, id))) {
            return;
        }

Coarse SEND first with no body read, then parse, then 400 on a malformed body, then the finer ANSWER check when turnId is present. That restores the pre-#687 ordering and keeps the finer action. No grant in Authz changed.

The call site is now pinned, and I verified the control myself

This is the part the first round missed. answerGatePasses was extracted so property 2 could be unit-tested, but a test on the helper says nothing about its caller — and the gate is behaviourally invisible today, because all three send grants are identical. So nothing could notice it disappearing.

Before the second commit, I deleted the call site and ran mvn -o -Dtest='FleetAppAuthTest,FleetAppTest' test:

Tests run: 56, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

After it, the same deletion:

[ERROR] Tests run: 16, Failures: 1 <<< FAILURE! -- in dev.ltms.fleet.rest.FleetAppAuthTest
[ERROR] FleetAppAuthTest.aGrantedTurnIdRequestAuditsBothSendAndAnswerButAPlainRequestAuditsSendAlone <<< FAILURE!
[ERROR] Tests run: 57, Failures: 1
BUILD FAILURE

Exactly one failure, and it is the new test. Restored afterwards; git diff on FleetApp.java empty.

The route it uses is the audit trail: allow() logs an allowed entry for every granted action except READ, METRICS and TASK_READ, and ANSWER is none of those, so a granted turnId request must log both SEND and ANSWER. The second control is in the same test — a plain request must log exactly List.of("SEND"), so the test cannot pass because something else happens to log ANSWER.

One number worth correcting

The worker reported 1946 tests and explained it as "1945 baseline + 1". The total was right for its own branch and the arithmetic was wrong: its branch is based on 2eb2d61 (1942) and adds 4 tests across its two commits, so 1942 + 4 = 1946. That it equals current main's own total is a coincidence, and the coincidence is what made the wrong arithmetic look right.

My build, trial-merging the branch onto current main (d0688c8, itself at 1946): 1950 tests, 0 failures, BUILD SUCCESS, exit=0, 173 *.xml report files. 1946 + 4 = 1950. Merged tree hash a5acb30112cd0b4a45c89dcb92446cfe54703549 equals the tree I built.

What remains unmeasured, unchanged from this ticket's filing

  • The DoS reading is still not measured. Nobody drove this with a real HTTP request, and nobody looked up Javalin 6.7.0's default maxRequestSize. The 1,000,000-byte figure is still one reviewer's claim, repeated. What is established is the ordering, read in the code and now pinned by a test.
  • maxRequestSize is still unset and still worth doing as its own change.
  • The two pre-existing items in the "also worth checking" section are untouched: the allow()/denyFor() audit-log asymmetry, and the malformed-body fallback. Both predate #687.

Why this mattered

#669 Unit B gives COLLABORATOR a different send grant. At that moment this ordering stops being cosmetic, and so does the ANSWER gate existing at all. Both are now in place and both are defended by a test that fails if they are removed.

## Fixed and merged. Closing. PR #694 merged to `main` as `edbd8d8`, pushed (`d0688c8..edbd8d8`). The shipped order in `FleetApp.sendMessage` is now: ```java private void sendMessage(Context ctx) { String id = ctx.pathParam("id"); if (!allow(ctx, routeAction("POST /sessions/{id}/message"), id)) { return; } JsonNode body; try { body = mapper.readTree(ctx.body()); } catch (Exception e) { body = null; } if (body == null) { ctx.status(400).json(Map.of("error", "bad_request", "detail", "body must be JSON")); return; } String turnId = body.path("turnId").asText(null); if (!answerGatePasses(turnId, action -> allow(ctx, action, id))) { return; } ``` Coarse `SEND` first with no body read, then parse, then 400 on a malformed body, then the finer `ANSWER` check when `turnId` is present. That restores the pre-#687 ordering and keeps the finer action. No grant in `Authz` changed. ## The call site is now pinned, and I verified the control myself This is the part the first round missed. `answerGatePasses` was extracted so property 2 could be unit-tested, but a test on the helper says nothing about its caller — and the gate is behaviourally invisible today, because all three send grants are identical. So nothing could notice it disappearing. **Before the second commit**, I deleted the call site and ran `mvn -o -Dtest='FleetAppAuthTest,FleetAppTest' test`: ``` Tests run: 56, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` **After it**, the same deletion: ``` [ERROR] Tests run: 16, Failures: 1 <<< FAILURE! -- in dev.ltms.fleet.rest.FleetAppAuthTest [ERROR] FleetAppAuthTest.aGrantedTurnIdRequestAuditsBothSendAndAnswerButAPlainRequestAuditsSendAlone <<< FAILURE! [ERROR] Tests run: 57, Failures: 1 BUILD FAILURE ``` Exactly one failure, and it is the new test. Restored afterwards; `git diff` on `FleetApp.java` empty. The route it uses is the audit trail: `allow()` logs an allowed entry for every granted action except `READ`, `METRICS` and `TASK_READ`, and `ANSWER` is none of those, so a granted `turnId` request must log both `SEND` and `ANSWER`. The second control is in the same test — a plain request must log exactly `List.of("SEND")`, so the test cannot pass because something else happens to log `ANSWER`. ## One number worth correcting The worker reported **1946 tests** and explained it as "1945 baseline + 1". The total was right for its own branch and the arithmetic was wrong: its branch is based on `2eb2d61` (1942) and adds 4 tests across its two commits, so 1942 + 4 = 1946. That it equals current `main`'s own total is a coincidence, and the coincidence is what made the wrong arithmetic look right. My build, trial-merging the branch onto current `main` (`d0688c8`, itself at 1946): **1950 tests, 0 failures, BUILD SUCCESS**, `exit=0`, 173 `*.xml` report files. 1946 + 4 = 1950. Merged tree hash `a5acb30112cd0b4a45c89dcb92446cfe54703549` equals the tree I built. ## What remains unmeasured, unchanged from this ticket's filing - **The DoS reading is still not measured.** Nobody drove this with a real HTTP request, and nobody looked up Javalin 6.7.0's default `maxRequestSize`. The 1,000,000-byte figure is still one reviewer's claim, repeated. What is established is the ordering, read in the code and now pinned by a test. - **`maxRequestSize` is still unset** and still worth doing as its own change. - The two pre-existing items in the "also worth checking" section are untouched: the `allow()`/`denyFor()` audit-log asymmetry, and the malformed-body fallback. Both predate #687. ## Why this mattered #669 Unit B gives `COLLABORATOR` a different send grant. At that moment this ordering stops being cosmetic, and so does the ANSWER gate existing at all. Both are now in place and both are defended by a test that fails if they are removed.
ltms closed this issue 2026-10-03 22:58:50 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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#689
Delete Branch "%!s()"

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?