feat(fleetnode): server-initiated discovery via ControlStream [PR 2/2]

[ankitgoswami]

PR 2 of a stack — stacks on top of #332 (PR 1: pairing + agent reporting). Layers operator-initiated discovery on top of the pairing + agent-reporting surface; builds on the existing fleetnodepairing.UpsertDiscoveredDevices ingestion path so the agent's batches flow through the registry to the operator's waiting stream.

Summary

• fleetnodecontrol.Registry — single-instance in-memory map of fleet_node_id → active ControlStream + per-command_id event channel. Newest-wins eviction, 64-slot buffered events, atomic droppedEvents counter, plus HasInFlightCommand for command-id binding.
• FleetNodeGateway.ControlStream — bidi handler. After Hello, registers the stream and pumps outgoing ControlCommand + incoming ControlAck through a side goroutine (buffer 2 to avoid linger on exit).
• ReportDiscoveredDevices command-id binding — rejects reports with empty or unknown command_id (FailedPrecondition). Only persists when an in-flight server-issued command matches the reporting fleet node.
• FleetNodeAdmin.DiscoverOnFleetNode — operator-facing server-streaming RPC. Validates CONFIRMED, normalizes IPRange to IPList (cap 4096), rejects MDNS, forwards IPList/Nmap. command_id via id.GenerateID(); payload via proto.Marshal. Bounded by a 5-minute per-command deadline.
• pairing.proto — buf.validate count caps on DiscoverRequest modes (4096 IPs, 256 ports per mode).
• ipscanner.GenerateIPsFromCIDR — exported for cross-package reuse.
• middleware/rpc_permissions — DiscoverOnFleetNode → fleetnode:manage.
• discovered_by_fleet_node_id immutable origin attribution — migration 000062 adds the column + index. Set once at first agent report, never cleared. GetActiveUnpairedDiscoveredDevices filters on IS NULL so the legacy cloud-side pairing flow never dials an IP a fleet node reported. UpsertDiscoveredDeviceFromFleetNode carries attribution + the per-row guard against cross-fleet-node overwrites.

Codex Security Review — round-2 findings addressed

• HIGH — UnpairDevice / RevokeFleetNode were clearing attribution. Split origin from ownership: fleet_node_device tracks ownership; discovered_by_fleet_node_id is immutable origin. Cloud pairing keeps refusing agent-reported IPs after unpair/revoke. (commit 83feac26)
• MEDIUM — publish/PublishAck channel-close race. Hold the mutex through the non-blocking send so cleanup/Unregister can't close the channel between lookup and send. New TestPublish_RaceWithCleanup exercises the path under -race. (commit 81aedebf)
• MEDIUM — DiscoverOnFleetNode could wait forever. Wrapped the operator ctx in context.WithTimeout(DiscoverCommandTimeout) (default 5min); returns DeadlineExceeded distinctly from operator cancel. New TestDiscoverOnFleetNode_TimesOutWhenAgentNeverResponds. (commit 81c225c6)

Deliberately deferred

• HIGH — normalizeDiscoverRequest doesn't validate scan targets (loopback / link-local / multicast / public IPs / hostnames / oversized Nmap CIDRs). Operator with fleetnode:manage is treated as a trusted role; the threat model assumes operators don't deliberately turn their own fleet node into a port scanner. Revisit if/when a less-trusted role gets fleet-node access.

Stack

• PR 1 (depends on): feat(fleetnode): device pairing + agent reporting (server) [PR 1/2] #332 — feat(fleetnode): device pairing + agent reporting (server)
• This PR retargets to main automatically when PR 1 merges.

Test plan

• cd server && go build ./... && go vet ./...
• just lint clean (buf, eslint, golangci-lint)
• DB_PASSWORD=fleet go test -count=1 -race across all affected packages — green
• CI green

🤖 Generated with Claude Code

[github-actions]

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

• Reviewed pull request diff only (9b5bf09537c612bc74af28d389f87f01d9174b0e...493c1575371181334d120665463aadf58e0e06d5, exact PR three-dot diff)
• Model: gpt-5.5

💡 Click "edited" above to see previous reviews for this PR.

---

Review Summary

Overall Risk: MEDIUM

Findings

[MEDIUM] Fleet-node reports can claim un-attributed discovered devices

• Category: Network Discovery
• Location: server/sqlc/queries/fleetnodepairing.sql:57
• Description: The fleet-node upsert allows a reporting node to update an existing discovered_device whenever discovered_by_fleet_node_id IS NULL, then sets attribution and endpoint fields from the report. Existing rows after migration, and server-local discoveries, are NULL-attributed. A compromised authenticated fleet node with any active discovery command can therefore report the identifier of an unpaired NULL-attributed device and claim/redirect it.
• Impact: Discovery poisoning and ownership confusion. The original row can be removed from server-local pairing views, attributed to the hostile node, and later paired/routed through the wrong fleet node.
• Recommendation: On conflict, only allow remote reports to update rows already attributed to the same fleet node. Handle legacy/NULL attribution through an explicit operator-approved claim path or strong immutable identity verification before setting attribution.

[MEDIUM] Remote-discovered devices are stranded from pairing

• Category: Reliability
• Location: server/internal/domain/pairing/service.go:1204
• Description: PairingService.Pair now refuses remote-fleet-node-attributed discovered rows and directs callers to PairDeviceToFleetNode, but the remote discovery path only persists discovered_device rows and streams pairing.Device objects. PairDeviceToFleetNode requires an existing device_id, and the changed unpaired-device queries exclude remote-attributed rows, so newly discovered fleet-node devices have no usable follow-up pairing path.
• Impact: Operators can see a miner from DiscoverOnFleetNode but cannot pair it unless a device row already exists through some other flow.
• Recommendation: Add an atomic remote-pair endpoint that accepts the attributed discovered_device/device_identifier, promotes it to device, and inserts fleet_node_device, or return a stable pairable database identifier from remote discovery and expose it in the admin flow.

[LOW] Server accepts remote scan sizes the fleet-node executor rejects

• Category: Reliability
• Location: server/internal/handlers/fleetnodeadmin/handler.go:277
• Description: DiscoverOnFleetNode expands IP ranges up to 4096 targets, matching the new proto cap, but the fleet-node executor hard-caps IP-list commands at 1024. Requests with 1025-4096 targets pass server validation, get dispatched, then fail on the agent and are surfaced as internal discovery failures.
• Impact: Valid-looking operator requests fail late and waste a command slot; the client gets an internal error instead of a validation error.
• Recommendation: Align server/proto and agent limits, or have the server enforce the agent’s actual maximum before dispatch. Also map agent BAD_REQUEST acks to InvalidArgument.

Notes

No cryptostealing/pool hijack logic, hardcoded payout addresses, SQL injection, shell command injection, or protobuf wire-format breakage was evident in the reviewed diff. Generated proto/sqlc outputs appear to be included with their source changes.

---

_{Generated by Codex Security Review |
Triggered by: @ankitgoswami |
Review workflow run}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44d1f14dad

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[Copilot]

Pull request overview

This PR wires up end-to-end fleet-node device discovery reporting and operator-driven device pairing. It introduces new gateway/admin RPCs, persists discovery attribution to the reporting fleet node, and adds an agent-side discovery loop that periodically scans CIDRs and reports discovered devices back to the server.

Changes:

• Add new RPCs: ReportDiscoveredDevices (gateway) and PairDeviceToFleetNode / UnpairDevice / ListFleetNodeDevices (admin), plus server auth interceptor allowlists.
• Add DB support for discovery attribution (discovered_device.discovered_by_fleet_node_id) and pairing queries + domain service/store for pairing operations.
• Add fleetnode agent discovery scanning loop with plugin-backed probing and unit tests.

Reviewed changes

Copilot reviewed 21 out of 32 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
server/sqlc/queries/fleetnodepairing.sql	New sqlc queries for discovery upsert, pairing/unpairing, and listing pairings.
server/sdk/v1/python/proto_fleet_sdk/generated/pb/driver_pb2_grpc.py	Regenerated Python gRPC stubs (version constant change).
server/migrations/000050_add_fleet_node_attribution_to_discovered_device.up.sql	Adds nullable discovered_by_fleet_node_id with FK + index for attribution.
server/migrations/000050_add_fleet_node_attribution_to_discovered_device.down.sql	Rollback for attribution column, FK, and index.
server/internal/handlers/interceptors/config.go	Adds new admin/gateway procedures to auth allowlists.
server/internal/handlers/fleetnodegateway/handler.go	Adds ReportDiscoveredDevices handler and injects pairing service into gateway handler.
server/internal/handlers/fleetnodegateway/handler_heartbeat_test.go	Updates handler construction to include pairing service dependency.
server/internal/handlers/fleetnodegateway/handler_discovery_test.go	New tests for discovery reporting persistence + missing subject rejection.
server/internal/handlers/fleetnodeadmin/handler.go	Adds pairing RPC handlers and injects pairing service dependency.
server/internal/handlers/fleetnodeadmin/handler_pairing_test.go	New integration tests covering admin pairing/unpair/list and auth gating.
server/internal/domain/stores/sqlstores/fleetnodepairing.go	New SQL store implementing pairing/discovery upsert operations.
server/internal/domain/fleetnodepairing/service.go	New domain service for pairing/listing and discovery upsert batching with error mapping.
server/internal/domain/fleetnodepairing/models.go	New domain models for discovery reports and pairing summaries.
server/internal/domain/fleetnodepairing/integration_test.go	Integration tests for service behavior (pair/unpair/list + discovery attribution).
server/generated/sqlc/models.go	Regenerated sqlc models to include DiscoveredByFleetNodeID.
server/generated/sqlc/fleetnodepairing.sql.go	Regenerated sqlc code for new pairing/discovery queries.
server/generated/sqlc/discovered_device.sql.go	Regenerated sqlc code to select new attribution column.
server/generated/sqlc/db.go	Regenerated sqlc statement preparation/closing for new queries.
server/generated/grpc/fleetnodegateway/v1/fleetnodegatewayv1connect/fleetnodegateway.connect.go	Regenerated Connect stubs for new gateway RPC.
server/generated/grpc/fleetnodeadmin/v1/fleetnodeadminv1connect/fleetnodeadmin.connect.go	Regenerated Connect stubs for new admin RPCs.
server/generated/grpc/fleetnodeadmin/v1/fleetnodeadmin.pb.go	Regenerated protobuf Go types for pairing RPCs/messages.
server/cmd/fleetnode/run.go	Adds discovery flags and runs discovery tick loop alongside heartbeat.
server/cmd/fleetnode/run_test.go	Extends stub gateway client to record discovery RPC calls/batches.
server/cmd/fleetnode/fake_gateway_test.go	Extends fake gateway server to handle ReportDiscoveredDevices.
server/cmd/fleetnode/discovery.go	Implements CIDR expansion, probing, and reporting logic (plugin-backed discoverer).
server/cmd/fleetnode/discovery_test.go	New unit tests for CIDR enumeration and discovery tick behavior/error handling.
server/cmd/fleetd/main.go	Wires new pairing service into server handlers.
proto/fleetnodegateway/v1/fleetnodegateway.proto	Adds ReportDiscoveredDevices RPC + request/response/messages.
proto/fleetnodeadmin/v1/fleetnodeadmin.proto	Adds pairing/admin RPCs + request/response/messages.
client/src/protoFleet/api/generated/fleetnodegateway/v1/fleetnodegateway_pb.ts	Regenerated TS API types/service for new gateway RPC.
client/src/protoFleet/api/generated/fleetnodeadmin/v1/fleetnodeadmin_pb.ts	Regenerated TS API types/service for new admin RPCs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 493c157537

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Align advertised scan limits with agent caps

When DiscoverOnFleetNode is used with an IP list/range above 1024 targets (or more than 10 ports), the request passes server-side validation because this proto cap is 4096 and the port caps are 256, but the fleetnode agent rejects it in server/cmd/fleetnode/control.go via maxIPsPerCommand = 1024 and maxPortsPerIP = 10. That makes operator requests that are valid according to the API consistently fail with an agent BAD_REQUEST ack instead of being rejected up front or chunked; keep these limits in sync or split the command before sending it to the agent.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Treat every failed ack as command failure

If a fleet node sends ControlAck{Succeeded:false} with an empty error_message (for example from a buggy/custom agent or a failure path that only sets code), this condition is false and the operator stream returns success even though the command explicitly failed. The handler should key off !Succeeded regardless of whether a message string is present, otherwise failed scans can be reported as successful with no batches or partial results.

Useful? React with 👍 / 👎.