Skip to content

fix(fleet): bind each node to the credential that heartbeats it - #1789

Merged
eldad-caura-ai merged 1 commit into
mainfrom
fix/fleet-node-binding
Oct 3, 2026
Merged

eldad-caura-ai merged 1 commit into
mainfrom
fix/fleet-node-binding

Conversation

@eldad-caura-ai

@eldad-caura-ai eldad-caura-ai commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Audit finding

Addresses M-85: fleet node identity was the caller-supplied node_name. Any agent or install credential in a tenant could heartbeat as another node, receive and ack its queued commands (deploy payloads included, which an agent credential may not queue itself), report results for any command by UUID, and list every command's payload and result.

Change

Each fleet node is now bound to the credential that heartbeats it.

  • Principal. A gateway-verified agent key binds as agent:<id> and an install credential as install:<uuid>. Every other credential is tenant-wide (tenant): tenant, user and admin keys, the shared CAURA_API_KEY, standalone mode, and an X-Agent-ID the caller asserted itself. OSS standalone and shared-key deployments therefore see no change. An install credential that arrives without its install UUID is refused (403 INSTALL_UUID_MISSING) rather than bound to a shared placeholder.
  • Heartbeat. fleet_upsert_node decides in its ON CONFLICT ... WHERE, against the locked row. A new or unbound node takes the caller's principal. A node bound to another narrow credential refuses a narrow caller with 403 FLEET_NODE_BOUND_TO_OTHER_CREDENTIAL, and the refused heartbeat changes nothing and delivers nothing. A tenant-wide credential is always admitted and takes the node back.
  • Command result and listing. A narrow credential reports on, and lists, only commands of nodes bound to it. Another node's command is the same 404 as a missing one.
  • Release. POST /api/v1/fleet/nodes/{node_id}/release (tenant credentials only) moves a node to a new credential, for example after rotating its key. With bind_agent_id or bind_install_uuid the node is bound to that credential in the same statement, so no other credential can claim it in between. Bare, it clears the binding and the next heartbeat binds the node.
  • Migration 055 adds nullable fleet_nodes.owner_principal, with no backfill. Existing nodes bind on their first heartbeat after deploy.

Trade-offs (approved policy)

  • Existing nodes bind on first heartbeat, and that heartbeat also receives the node's pending commands. Between deploy and each existing node's first heartbeat, a narrow credential in the same tenant that heartbeats first can claim the node and take what is queued. The tenant's own next heartbeat takes the node back; a node whose own credential is narrow is rebound with a release that names it. Nothing recorded which credential an existing node uses, and binding every existing node to the tenant instead would lock out nodes that heartbeat with agent keys or install credentials until an operator rebinds each one.
  • Moving a node to a different narrow credential takes a release that names it. A bare release reopens the first-heartbeat window for that node.
  • Deploy core-storage-api (migration 055) before core-api, as the pipeline already does. A new core-api sends owner_principal, which an older storage cannot write.

Not in this PR

  • Interview fleet-node windows (the rest of M-86) follow in a separate PR, now that caura PR fix(interview): accept adapter streams and bind them to their agent #1788 has merged: a window for a fleet node must cite a delivered interview_request, used once. It relies on this PR's listing filter, which stops an agent credential reading another node's pending command ids.
  • The enterprise dashboard has no button for the release endpoint yet.

Verification

  • Storage tests cover bind, refuse-without-change, refresh, reclaim, legacy unbound rows, caller-omitted binding, release (including tenant scoping and binding straight to a named credential), and the result and listing filters. Route tests cover the drain attack, over-refusal guards for own nodes and asserted headers, install credentials (including one with no install UUID), release authorization, release that binds a named agent key or install credential with no window for a squatter, at most one bind target, and 404 for an unknown node.
  • Tests first: the tests-only push (run 37074834988) failed all 10 attack tests in tests/test_fleet_node_binding.py against unfixed main, with 8459 passing. The drain test's response carried the victim node's queued command. The two over-refusal guards passed, as they must without the fix. That step stopped the job before the storage suite ran. The fix is now amended into the same single commit.
  • No repository scripts, dependency installs, tests or builds were run on Eldad's Mac under its safety hold. CI runs the suites.

One signed-off commit. Do not merge or enable auto-merge; Eldad merges after review.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Claude Code Review — skipped: draft PR

@eldad-caura-ai
eldad-caura-ai marked this pull request as ready for review October 2, 2026 23:18
@eldad-caura-ai
eldad-caura-ai requested a review from a team as a code owner October 2, 2026 23:18
@eldad-caura-ai
eldad-caura-ai force-pushed the fix/fleet-node-binding branch from 007c2d3 to ebd1b86 Compare October 2, 2026 23:18
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

🤖 Review by Claude Code

Claude Code Review ✅ No issues found.


Reviewed by claude-sonnet-5-5 · cost $0.125535

@eldad-caura-ai
eldad-caura-ai force-pushed the fix/fleet-node-binding branch from ebd1b86 to 5ad92a7 Compare October 2, 2026 23:19
@eldad-caura-ai

Copy link
Copy Markdown
Member Author

@erni-a pelrq ppaove

erni-a
erni-a previously approved these changes Oct 2, 2026
@eldad-caura-ai

Copy link
Copy Markdown
Member Author

@claude

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

🤖 Review by Claude Code

Summary

This PR binds each fleet node to the credential that heartbeats it. It adds an owner_principal column and enforces the binding inside the upsert's ON CONFLICT … WHERE, so the check and the bind are atomic. It also filters command results and listings to the caller's bound nodes, and adds a tenant-only release route. The design is sound and I found no correctness or security regressions in the storage logic. The one problem is a defensive fallback in the principal derivation that fails open.

Medium/Low Issues

Install credentials without a UUID share one binding principal

Severity: Low
File: core-api/src/core_api/routes/fleet.py:163-178
Problem: _node_principal falls back to the literal install:unknown when auth.install_uuid is empty, so any two such install credentials are treated as the same owner and can act as each other's nodes.

🤖 Claude Code Prompt
In core-api/src/core_api/routes/fleet.py, function `_node_principal` (around lines 163-178), the install-credential branch builds the principal as "install:" followed by `auth.install_uuid or 'unknown'`.

If an install credential ever arrives with an empty or None `install_uuid`, every such credential shares the principal "install:unknown". That defeats the per-credential isolation M-85 adds.

Change the branch so it never collapses distinct credentials into one shared principal. The safest option is to refuse the request when `is_install_credential` is true but `install_uuid` is missing. Do this by raising an HTTPException 403 with a coded detail, or by having `_node_principal` raise.

The alternative is to guarantee that `install_uuid` is always populated for install credentials. If you choose that, add a defensive assertion or log here instead of the 'unknown' fallback.

Keep the existing behaviour for agent and tenant credentials.

Add a test in tests/test_fleet_node_binding.py covering an install credential with no uuid.

Reviewed by claude-sonnet-5-5 · cost $0.4199339999999999

@eldad-caura-ai
eldad-caura-ai force-pushed the fix/fleet-node-binding branch from e64c431 to 7afc45e Compare October 2, 2026 23:40
@eldad-caura-ai

Copy link
Copy Markdown
Member Author

Addressed the Low finding in 7afc45e, still a single signed-off commit. _node_principal no longer falls back to install:unknown. An install credential without an install UUID now gets a 403 with the new code INSTALL_UUID_MISSING on heartbeat, command result and command listing, so no two such credentials can share a binding. The release route already refused install credentials. New test: test_an_install_credential_without_a_uuid_is_refused. Without the fix the same heartbeat returns 200 and binds the shared principal.

🤖 Addressed by Claude Code

@eldad-caura-ai
eldad-caura-ai force-pushed the fix/fleet-node-binding branch from 7afc45e to 17272a2 Compare October 3, 2026 10:00
@eldad-caura-ai

Copy link
Copy Markdown
Member Author

@claude

@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

🤖 Review by Claude Code

Summary

This PR binds each fleet node to the credential that heartbeats it. It adds an owner_principal column (migration 055), makes the binding check atomic in the upsert's conflict arm, and filters command results and listings to the caller's own nodes. The check is applied correctly on the storage side. I found one design gap that leaves part of the original attack open. I did not review the changes under tests/ and core-storage-api/tests/, per the review scope.

Medium/Low Issues

Unbound nodes can be claimed by any narrow credential, so the queue-drain attack still works after migration 055 or a release

Severity: Medium
File: core-storage-api/src/core_storage_api/services/postgres_service.py:12010-12030 (also core-api/src/core_api/routes/fleet.py:~893-935, core-storage-api/.../versions/055_fleet_node_owner_principal.py)
Problem: A node with NULL owner_principal is bound by whichever narrow credential heartbeats it first, and that heartbeat also returns and acks the node's pending commands, deploy payloads included. This covers every pre-existing node right after 055 (no backfill) and every node after /release. During that window an agent or install credential in the same tenant can claim the node, drain its queue, and lock out the real owner with a 403 until a tenant credential releases it. This is the M-85 attack, just narrowed to the unbound window.

🤖 Claude Code Prompt
Context: M-85 fleet node binding. In core-storage-api/src/core_storage_api/services/postgres_service.py, fleet_upsert_node (around lines 11993-12030) lets a narrow principal (agent:<id> / install:<uuid>) bind any node whose owner_principal is NULL. The conflict-arm WHERE is `owner_principal IS NULL OR owner_principal = <caller>`. In core-api/src/core_api/routes/fleet.py, heartbeat() then calls get_pending_commands and ack_commands for that node. As a result, right after migration 055 (no backfill), or right after POST /fleet/nodes/{node_id}/release, any narrow credential in the tenant can claim an unbound node by name, receive and ack its queued commands (including deploy payloads), and lock out the real owner.

Please reduce this window without breaking the documented flows. Options, pick one and document it:
(a) Do not let a narrow credential claim an existing row with NULL owner_principal and a non-NULL created_at/last_heartbeat. Only a tenant-wide credential may adopt unbound pre-existing nodes, while a narrow credential may still create a new node or bind one it just inserted.
(b) Or have release_node (core-api/src/core_api/routes/fleet.py ~1241 and the storage fleet_release_node) accept an optional target principal to bind the node to directly, instead of clearing it to NULL, so rotation has no open window.
(c) At minimum, in heartbeat(), do not deliver or ack pending commands on the heartbeat that first binds a narrow credential to a previously unbound existing node.

Update the docstring of fleet_upsert_node, the 055 migration docstring, and docs/api-reference.md to describe whichever behaviour you choose. Add tests in tests/test_fleet_node_binding.py and core-storage-api/tests/test_fleet_node_owner_binding.py covering a narrow credential heartbeating an unbound existing node that has queued commands.

Reviewed by claude-sonnet-5-5 · cost $0.5732772500000001

@eldad-caura-ai
eldad-caura-ai marked this pull request as draft October 3, 2026 12:06
@eldad-caura-ai
eldad-caura-ai force-pushed the fix/fleet-node-binding branch 2 times, most recently from 3158034 to e9ddad7 Compare October 3, 2026 13:10
A fleet node was keyed on its caller-supplied node_name alone, so any agent or install credential in a tenant could heartbeat as another node, receive and ack its queued commands (deploy payloads included), report results for its commands and list every node's command payloads (M-85). Migration 055 adds fleet_nodes.owner_principal. fleet_upsert_node binds a new or unbound node to a gateway-verified agent key or install credential in its ON CONFLICT WHERE and refuses a different narrow credential without changing the row; core-api returns 403 FLEET_NODE_BOUND_TO_OTHER_CREDENTIAL. Tenant-wide credentials, including the shared key and caller-asserted X-Agent-ID, still act as any node and take a node back. An install credential without an install UUID is refused rather than sharing a placeholder binding. Command results and listings narrow to the caller's bound nodes. POST /api/v1/fleet/nodes/{node_id}/release lets a tenant credential move a node to a new credential: with bind_agent_id or bind_install_uuid it binds the node at once, leaving no window for another credential, and bare it clears the binding for the next heartbeat. Existing nodes bind on their first heartbeat after the migration.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: eldad-caura <eldad@caura.ai>
@eldad-caura-ai
eldad-caura-ai force-pushed the fix/fleet-node-binding branch from e9ddad7 to b42b574 Compare October 3, 2026 13:10
@eldad-caura-ai
eldad-caura-ai marked this pull request as ready for review October 3, 2026 13:20
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

🤖 Review by Claude Code

Claude Code Review ✅ No issues found.


Reviewed by claude-sonnet-5-5 · cost $0.45248449999999996

@eldad-caura-ai

Copy link
Copy Markdown
Member Author

@erni-a ready for human review. CI, CodeQL, DCO and a fresh Claude review (no issues found) all pass on the single commit b42b574. Since the last review:

  • the shared install:unknown fallback is gone: an install credential with no install UUID now gets 403 INSTALL_UUID_MISSING;
  • release can bind a node straight to a named credential (bind_agent_id / bind_install_uuid), so a key rotation leaves no window for another credential;
  • the remaining window is documented: existing nodes bind on their first heartbeat after migration 055.

Both additions were tests-first: the tests-only run 37121821097 failed only the 3 new release tests. Deploy core-storage-api (migration 055) before core-api. Not auto-merging; Eldad merges.

🤖 Posted by Claude Code

@eldad-caura-ai
eldad-caura-ai merged commit 078b2c1 into main Oct 3, 2026
18 checks passed
@eldad-caura-ai
eldad-caura-ai deleted the fix/fleet-node-binding branch October 3, 2026 13:25
@caura-deploy-bot caura-deploy-bot Bot mentioned this pull request Oct 3, 2026
eldad-caura-ai pushed a commit that referenced this pull request Oct 3, 2026
🤖 I have created a release *beep* *boop*
---


<details><summary>backend: 3.21.1</summary>

##
[3.21.1](backend-v3.21.0...backend-v3.21.1)
(2026-10-03)


### Bug Fixes

* **fleet:** bind each node to the credential that heartbeats it
([#1789](#1789))
([078b2c1](078b2c1))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: release-please[bot] <release-please[bot]@users.noreply.github.com>
Co-authored-by: caura-deploy-bot[bot] <265395343+caura-deploy-bot[bot]@users.noreply.github.com>
eldad-caura-ai added a commit that referenced this pull request Oct 3, 2026
…to it (#1792)

## Audit finding

Finishes **M-86**. caura PR #1788 bound adapter streams (`cc:` /
`cursor:` node ids) to the agent that advances them. Fleet-node windows
were still open: an agent or install credential could submit a window
for any fleet node of its tenant by naming the node and any
`command_id`. That advanced the node's watermark, which only moves
forward, and wrote its job doc.

## Change

- **Gate.** A window for a fleet node (UUID `node_id`) from an agent or
install credential must cite the `interview_request` the scheduler
queued for that node, and the node's heartbeat must have delivered it
(`acked`). Anything else gets 403 `INTERVIEW_REQUEST_REQUIRED` before
the watermark or job doc is touched. This is the same "agent or install
credential" test #1788 applies to adapter streams.
- **Used once.** Storage claims the request in one conditional UPDATE:
this tenant, this node, `interview_request`, `acked`, no `result` yet.
The claim writes a marker into `result`, so a second window citing the
same id is refused. A used id stops being secret, because the job doc
and watermark record it. The node's own result report still overwrites
the marker and closes the command.
- **Unchanged.** Tenant credentials keep tenant-wide authority, as they
do for adapter streams. The plugin already submits once per delivered
command and cites its id (`plugin/src/heartbeat.ts`), and the scheduler
only looks at `pending` commands, so neither changes. A failed submit is
retried by the scheduler as a fresh command, as before.

## Why this relies on caura PR #1789

The request id is the credential here. Since #1789, an agent or install
credential can only list or receive commands for nodes bound to it, so
it cannot read another node's pending `interview_request` before that
node uses it.

## Verification

- Route tests cover an uninvited agent and an uninvited install, a
queued but undelivered request, one window per request, a request for
another node, and a guard that tenant credentials still submit directly.
The existing attribution test now cites a delivered request.
- Storage tests cover claim once, undelivered, wrong node, wrong tenant,
non-interview command, an already completed request, and the node's
result report closing a claimed request.
- Tests first: the tests-only push ([run
37126305662](https://github.com/caura-ai/caura/actions/runs/37126305662/job/111212158891))
failed exactly the five attack tests against `main`, with 8644 passing.
In each one the unfixed route answered `accepted` and moved the node's
watermark. The tenant guard and the updated attribution test passed.
That step stopped the job before the storage suite ran. The fix is now
amended into the same single commit.
- No repository scripts, dependency installs, tests or builds were run
on Eldad's Mac under its safety hold. CI runs the suites.

One signed-off commit. Do not merge or enable auto-merge; Eldad merges
after review.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: eldad-caura <eldad@caura.ai>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants