Download miner logs via fleet nodes#599
Conversation
🔐 Codex Security Review
Review SummaryOverall Risk: NONE FindingsNo security, correctness, or reliability findings in the scoped diff. NotesReviewed The new miner log artifact path includes the expected controls: authenticated fleet-node subject lookup, in-flight command/artifact expectation binding, per-node upload limits, size and SHA-256 validation before storing artifacts, max-size enforcement for miner logs, and CSV formula neutralization before batch log materialization. I did not run tests; this was a static review of the supplied PR diff. Generated by Codex Security Review | |
56ed6a3 to
92d5054
Compare
There was a problem hiding this comment.
Pull request overview
Enables the existing ProtoFleet “DownloadLogs / GetCommandBatchLogBundle” workflow to work for miners paired through fleet nodes by having fleet nodes upload a MINER_LOGS command artifact, and having the server materialize that artifact into the same batch logs directory used for directly-connected miners.
Changes:
- Adds a fleet-node
DownloadLogsActionand implements node-side log download → CSV formatting → artifact upload, with size limiting. - Extends the fleet-node control registry to return completed upload refs with the terminal ack and to fail “OK” acks that arrive before expected uploads complete.
- Adds server-side artifact materialization into the existing batch log directory (plus tests) and centralizes CSV formatting in
miner/logformat(including CSV formula neutralization).
Reviewed changes
Copilot reviewed 20 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| server/internal/infrastructure/files/service.go | Adds SaveCommandArtifactLog to copy/verify a validated command artifact into the batch logs directory. |
| server/internal/infrastructure/files/service_test.go | Adds coverage for artifact log materialization and bundling behavior (single + mixed + error cases). |
| server/internal/handlers/fleetnode/gateway/handler.go | Plumbs SizeBytes into artifact expectations and maps “too large” admission failures to ResourceExhausted. |
| server/internal/handlers/fleetnode/gateway/handler_artifact_test.go | Tests rejecting oversized miner log uploads and allowing retry with bounded size. |
| server/internal/domain/plugins/plugin_miner.go | Switches direct-miner log CSV formatting to shared logformat. |
| server/internal/domain/plugins/plugin_miner_test.go | Updates tests to use logformat.FormatLineToCSVRow. |
| server/internal/domain/miner/service.go | Injects files service into remote-node miner config for log artifact materialization. |
| server/internal/domain/miner/remotenode/miner.go | Implements DownloadLogs for remote-node miners using artifact-result command send + artifact materialization. |
| server/internal/domain/miner/remotenode/miner_test.go | Adds tests for remote-node DownloadLogs action dispatch + artifact requirement behavior. |
| server/internal/domain/miner/logformat/logformat.go | New shared CSV formatter (streaming + row-based) with CSV formula neutralization and a max-bytes constant. |
| server/internal/domain/miner/logformat/logformat_test.go | Tests streaming formatter equivalence and formula neutralization behavior. |
| server/internal/domain/fleetnode/control/stream.go | Enforces per-expectation max-size on admitted artifact transfers. |
| server/internal/domain/fleetnode/control/registry.go | Adds size fields to artifact expectations and introduces ErrArtifactTooLarge. |
| server/internal/domain/fleetnode/control/registry_test.go | Tests for returning completed upload refs and rejecting OK acks before uploads complete. |
| server/internal/domain/fleetnode/control/command.go | Adds SendCommandWithArtifactResults and snapshots upload refs before removing in-flight command state. |
| server/cmd/fleetnode/minercommand.go | Implements node-side DownloadLogs action to format CSV, upload artifact, and only then ack success. |
| server/cmd/fleetnode/minercommand_test.go | Adds tests covering successful uploads, upload failure, partial-data rejection, and size limiting. |
| server/cmd/fleetnode/fake_gateway_test.go | Extends fake gateway to capture artifact upload streams and return refs/errors. |
| server/cmd/fleetnode/control.go | Passes gateway client into miner-command handler so DownloadLogs can upload artifacts. |
| proto/fleetnodegateway/v1/fleetnodegateway.proto | Adds DownloadLogsAction to the internal fleet-node miner command envelope. |
| client/src/protoFleet/api/generated/fleetnodegateway/v1/fleetnodegateway_pb.ts | Regenerated TS output for the proto change (generated). |
05d4b09 to
090eecd
Compare
- Treat materialized partial log artifacts as terminal to avoid duplicate retries - Gate remote log downloads to artifact upload capacity per fleet node
Return BAD_REQUEST when log size limits are hit before artifact upload so the command fails terminally instead of retrying a PARTIAL ack with no artifact.
Build the fleetnode-ui-test image before the one-shot enrollment helper runs so local manual tests cannot reuse a stale binary that omits the required encryption_pubkey.
Materialize any uploaded partial artifact, but return a FailedPrecondition so incomplete miner logs are not marked successful or retried.
Map DownloadLogs BAD_REQUEST acks to FailedPrecondition so miner-log size-limit failures do not burn queue retries.
Open batch log files with O_EXCL and retry with a numeric suffix so same-second MAC collisions cannot truncate earlier miner logs.
Reviewable diff: +425/-155 across 9 files (excludes generated, test, and story files).
Summary
Operators can now download miner logs from miners paired through fleet nodes using the existing ProtoFleet
DownloadLogsandGetCommandBatchLogBundleflow. Fleet nodes collect the logs from their paired miners, upload the CSV as a command artifact, and the server materializes that artifact into the same batch log directory used by direct miners. The browser-facing output remains the existing single CSV or ZIP bundle, with no new client UX or publicminercommand.v1API surface.How it works
The operator starts the existing download-logs command. For remote-node miners, the server sends a fleet-node
DownloadLogsActioncontaining the batch UUID and registers one expectedMINER_LOGSupload for the target device identifier.The fleet node receives the command, downloads logs from the SDK device, formats them with the same CSV formatter used by direct plugin miners, uploads the CSV through
UploadCommandArtifact, and only then acknowledges success. The gateway-side control registry returns completed upload refs before removing the in-flight command, so the remote miner adapter can require the uploaded ref and copy the validated artifact intologs/<batchUUID>/using the same miner-log filename convention as direct miners.The existing batch finalizer then bundles whatever files are in the batch directory. Direct-only, fleet-node-only, and mixed batches all keep the current CSV-for-single-device and ZIP-for-multiple-devices behavior.
Diagrams
Areas of the code involved
proto/fleetnodegateway/v1DownloadLogsActionto the internal fleet-node command envelopeserver/internal/domain/fleetnode/controlserver/internal/domain/miner/remotenodeDownloadLogsAction, expects oneMINER_LOGSupload scoped to the device, and materializes the returned artifactserver/cmd/fleetnodeDownloadLogsAction, downloads device logs, formats CSV, uploads command artifacts, and acks only after upload successserver/internal/domain/miner/logformatandpluginsserver/internal/infrastructure/filesKey technical decisions & trade-offs
minercommand.v1unchanged; fleet-node log download support rides on the internalfleetnodegateway.v1command payload instead of adding a new operator-facing API.Testing & validation
buf lint(cd server && golangci-lint run -c .golangci.yaml)(cd server && go test ./cmd/fleetnode ./internal/infrastructure/files ./internal/domain/miner/logformat ./internal/domain/plugins ./internal/domain/fleetnode/control ./internal/domain/miner/remotenode)git diff --checkNot covered locally: full
just lintstill needs client dependencies installed (eslintwas missing fromclient/node_modules), and full DB-backed server tests were not rerun because local Postgres auth for userfleetfailed in earlier attempts.