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

One audit-skip rule, two hand-maintained lists: FleetMcp.denyFor and FleetApp.allow disagree on METRICS, and only a grep returning zero keeps it harmless #700

New Issue
Open
opened 2026-10-04 00:49:08 +02:00 by ltms · 0 comments
ltms commented 2026-10-04 00:49:08 +02:00
Owner
Copy Link
Copy Source

Carried forward from #669 Unit C, where it was found by the implementer's shape sweep and then left unfiled by two leads. Measured by me in the main clone at b92a669; each command is next to its number.

The two lists

Both gates suppress the audit allowed entry for read-shaped actions, and both explain it with the same comment — reads would drown the trail. They do not suppress the same set.

fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java:693:

if (Authz.permits(caller, action, target, Authz.NO_KNOWN_LEAD_OR_COLLABORATOR)) {
    if (action != Authz.Action.READ && action != Authz.Action.TASK_READ) {
        AuditLog.allowed(caller, action, target); // reads would drown the trail
    }
    return null;
}

fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java:294:

if (permitsFor(caller, action, target)) {
    if (action != Authz.Action.READ && action != Authz.Action.METRICS
            && action != Authz.Action.TASK_READ) {
        AuditLog.allowed(caller, action, target); // reads would drown the trail
    }
    return true;
}

So REST skips METRICS; MCP does not.

Why it has no effect today, with the control

Action.METRICS never appears in FleetMcp, so no MCP tool ever reaches denyFor with it:

Action.METRICS       0
Action.READ          2
Action.TASK_READ     4
Action.SEND          2

(grep -c "<pattern>" fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java || true, run per pattern.)

The three non-zero rows are the point. A zero on its own reads the same whether the action is genuinely absent or the pattern is wrong, so the other three are there to show the pattern style matches when there is something to match. METRICS is really absent.

Why it is still worth a ticket

This is one invariant — do not write an allowed entry for a read — enforced by two literal lists that must be edited together, in two files, with nothing connecting them. Today the divergence is masked by an absence, not by a check. The day any fleet_* tool passes Action.METRICS to denyFor, that tool starts writing audit entries the REST route deliberately suppresses, and the trail quietly differs by surface. Nothing fails, and no test asks the question.

Note the direction: the masking fact is about FleetMcp's call sites, while the defect is in denyFor's list. A future call site is exactly what removes the mask, so "no live effect" is a statement about today's callers and not a property of the gate.

What would fix it

Ask Authz.Action whether it is read-shaped — one predicate on the enum, or one shared constant set — and have both gates call it. That is the "share the computation, not the inputs" fix CallerResolver.isLoopback already carries a javadoc about (#305): two copies of one rule drifted there too, and sharing the inputs would not have prevented it.

A test then has something to pin: for every Authz.Action value, the MCP gate and the REST gate agree on whether an allowed entry is written. Enumerating the enum means a new action cannot be added to one list and forgotten in the other.

Not verified by me

  • Whether any REST route actually passes METRICS today. I measured the FleetMcp side only, because that is where the shorter list is.
  • Whether AuditLog has a cheaper seam for this than a predicate on the enum. I did not read it.
  • I have not checked whether denied entries have the same split; the two blocks above only cover the allowed path.
Carried forward from #669 Unit C, where it was found by the implementer's shape sweep and then left unfiled by two leads. Measured by me in the main clone at `b92a669`; each command is next to its number. ## The two lists Both gates suppress the audit `allowed` entry for read-shaped actions, and both explain it with the same comment — `reads would drown the trail`. They do not suppress the same set. `fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java:693`: ```java if (Authz.permits(caller, action, target, Authz.NO_KNOWN_LEAD_OR_COLLABORATOR)) { if (action != Authz.Action.READ && action != Authz.Action.TASK_READ) { AuditLog.allowed(caller, action, target); // reads would drown the trail } return null; } ``` `fleetd/src/main/java/dev/ltms/fleet/rest/FleetApp.java:294`: ```java if (permitsFor(caller, action, target)) { if (action != Authz.Action.READ && action != Authz.Action.METRICS && action != Authz.Action.TASK_READ) { AuditLog.allowed(caller, action, target); // reads would drown the trail } return true; } ``` So REST skips `METRICS`; MCP does not. ## Why it has no effect today, with the control `Action.METRICS` never appears in `FleetMcp`, so no MCP tool ever reaches `denyFor` with it: ``` Action.METRICS 0 Action.READ 2 Action.TASK_READ 4 Action.SEND 2 ``` (`grep -c "<pattern>" fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java || true`, run per pattern.) The three non-zero rows are the point. A zero on its own reads the same whether the action is genuinely absent or the pattern is wrong, so the other three are there to show the pattern style matches when there is something to match. `METRICS` is really absent. ## Why it is still worth a ticket This is one invariant — *do not write an `allowed` entry for a read* — enforced by two literal lists that must be edited together, in two files, with nothing connecting them. Today the divergence is masked by an absence, not by a check. The day any `fleet_*` tool passes `Action.METRICS` to `denyFor`, that tool starts writing audit entries the REST route deliberately suppresses, and the trail quietly differs by surface. Nothing fails, and no test asks the question. Note the direction: the masking fact is about `FleetMcp`'s *call sites*, while the defect is in `denyFor`'s *list*. A future call site is exactly what removes the mask, so "no live effect" is a statement about today's callers and not a property of the gate. ## What would fix it Ask `Authz.Action` whether it is read-shaped — one predicate on the enum, or one shared constant set — and have both gates call it. That is the "share the computation, not the inputs" fix `CallerResolver.isLoopback` already carries a javadoc about (#305): two copies of one rule drifted there too, and sharing the inputs would not have prevented it. A test then has something to pin: for every `Authz.Action` value, the MCP gate and the REST gate agree on whether an `allowed` entry is written. Enumerating the enum means a new action cannot be added to one list and forgotten in the other. ## Not verified by me - Whether any REST route actually passes `METRICS` today. I measured the `FleetMcp` side only, because that is where the shorter list is. - Whether `AuditLog` has a cheaper seam for this than a predicate on the enum. I did not read it. - I have not checked whether `denied` entries have the same split; the two blocks above only cover the `allowed` path.
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#700
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?