Add server-side orbit debug logging enablement - currently only configurable as a duration-after-enrollment setting#45367
Conversation
GitHub Actions
Inactive
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #45367 +/- ##
==========================================
- Coverage 66.81% 66.72% -0.10%
==========================================
Files 2724 2729 +5
Lines 219027 218490 -537
Branches 10716 10716
==========================================
- Hits 146342 145779 -563
+ Misses 59521 59482 -39
- Partials 13164 13229 +65
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
GitHub Actions
Inactive
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
GitHub Actions
Inactive
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR implements server-driven Orbit debug logging on enrollment. It adds the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds a server-driven mechanism to enable Orbit debug logging for a limited time after enrollment, based on an orbit.debug_logging_on_enroll_duration agent option. It introduces a per-host orbit_debug_until stamp, surfaces a debug_logging field in the Orbit config response, and updates Orbit to reconcile osquery flags and its own log level accordingly.
Changes:
- Add
orbit_debug_untiltohostsand a datastore method to extend it idempotently. - On Orbit enrollment, stamp
orbit_debug_untilfrom effective agent options; on config fetch, emitdebug_loggingand mergeverbose=trueintocommand_line_startup_flagswhile active. - Update Orbit to (a) reconcile nil/empty
Flagsto clearosquery.flags, (b) preserve startup--debugas a floor for osquery flags, and (c) toggle Orbit zerolog level via newDebugLogReceiver, with tests.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/orbit.go | Stamps orbit_debug_until on enroll; merges debug behavior into Orbit config response. |
| server/service/orbit_test.go | Adds unit tests for stamping and debug flag/flag-merge logic. |
| server/service/integration_core_test.go | Integration test covering validation + stamping behavior on enroll. |
| server/mock/datastore_mock.go | Adds mock support for ExtendHostOrbitDebugUntil. |
| server/fleet/orbit.go | Adds DebugLogging *bool to OrbitConfig payload. |
| server/fleet/hosts.go | Adds OrbitDebugUntil field to Host. |
| server/fleet/datastore.go | Adds ExtendHostOrbitDebugUntil to datastore interface. |
| server/fleet/agent_options.go | Adds orbit.debug_logging_on_enroll_duration option + validation caps. |
| server/fleet/agent_options_test.go | Adds validation test cases for new orbit agent option. |
| server/datastore/mysql/schema.sql | Adds orbit_debug_until column to hosts schema snapshot. |
| server/datastore/mysql/migrations/tables/20260512143542_AddOrbitDebugUntilToHosts.go | Migration to add orbit_debug_until. |
| server/datastore/mysql/migrations/tables/20260512143542_AddOrbitDebugUntilToHosts_test.go | Migration test verifying column behavior (NULL/set/clear). |
| server/datastore/mysql/hosts.go | Loads orbit_debug_until; adds ExtendHostOrbitDebugUntil implementation. |
| server/datastore/mysql/hosts_test.go | Tests idempotent “extend only” semantics for orbit_debug_until. |
| orbit/pkg/update/flag_runner.go | Allows reconciling nil/empty flags; preserves startup debug as a floor. |
| orbit/pkg/update/flag_runner_test.go | Tests nil-flags reconciliation and startup-debug floor behavior. |
| orbit/pkg/update/debug_log_runner.go | New receiver to toggle Orbit zerolog level from server config. |
| orbit/pkg/update/debug_log_runner_test.go | Tests debug log receiver behavior + startup floor. |
| orbit/cmd/orbit/orbit.go | Registers debug log receiver; plumbs StartedInDebug into flag runner. |
| orbit/changes/43997-orbit-debug-logging-on-enroll | Orbit changelog entry for new agent option. |
| changes/43997-orbit-debug-logging-on-enroll | Server changelog entry for new agent option. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/service/integration_core_test.go (1)
17031-17045: 💤 Low valueConsider testing the exact cap boundary (86400).
The test verifies that 86401 is rejected but doesn't confirm that the maximum allowed value (86400 = 1 day) is accepted. Testing the exact boundary would strengthen validation coverage.
🧪 Optional test addition
// Reject above cap. var acResp appConfigResponse s.DoJSON("PATCH", "/api/latest/fleet/config", json.RawMessage(`{ "agent_options": { "orbit": {"debug_logging_on_enroll_duration": 86401} } }`), http.StatusBadRequest, &acResp) + + // Accept exact cap (86400 = 1 day). + s.DoJSON("PATCH", "/api/latest/fleet/config", json.RawMessage(`{ + "agent_options": { "orbit": {"debug_logging_on_enroll_duration": 86400} } + }`), http.StatusOK, &acResp) // Reject negative.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/service/integration_core_test.go` around lines 17031 - 17045, Add a positive boundary test for the Orbit debug logging cap: call s.DoJSON with the PATCH to "/api/latest/fleet/config" using agent_options.orbit.debug_logging_on_enroll_duration set to 86400 and assert success (http.StatusOK) populating appConfigResponse; this complements the existing negative tests for 86401 and -1 and verifies that the exact allowed maximum is accepted. Ensure you use the same s.DoJSON helper and appConfigResponse type as in the surrounding tests so the new assertion integrates consistently.server/datastore/mysql/migrations/tables/20260512143542_AddOrbitDebugUntilToHosts_test.go (1)
25-25: ⚡ Quick winUse a unique host identifier in test queries.
These statements target a single host via
hostname, which is not unique onhosts; usingnode_keyorosquery_host_idwould keep the test deterministic if fixtures/data shape changes.As per coding guidelines: “ensure that appropriate filtering criteria are applied… check for missing WHERE clauses or incorrect filtering that could lead to incorrect or non-deterministic results.”
Also applies to: 31-31, 34-34, 40-40, 43-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/datastore/mysql/migrations/tables/20260512143542_AddOrbitDebugUntilToHosts_test.go` at line 25, The test uses non-unique hostname filtering in queries like the SELECT that scans into debugUntil; change these to use a unique host identifier (e.g., node_key or osquery_host_id) instead of hostname so the test is deterministic—update the SQL WHERE clause(s) in the queries that call db.QueryRow(`SELECT orbit_debug_until FROM hosts WHERE hostname = ?`, "host-1") and the other affected queries (the ones at the other noted locations) to use WHERE node_key = ? (or osquery_host_id = ?) and pass the fixture's unique node_key value; ensure any variables/helpers referencing the host (e.g., the test fixture that provides "host-1") are updated to supply the unique identifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@server/datastore/mysql/migrations/tables/20260512143542_AddOrbitDebugUntilToHosts_test.go`:
- Line 25: The test uses non-unique hostname filtering in queries like the
SELECT that scans into debugUntil; change these to use a unique host identifier
(e.g., node_key or osquery_host_id) instead of hostname so the test is
deterministic—update the SQL WHERE clause(s) in the queries that call
db.QueryRow(`SELECT orbit_debug_until FROM hosts WHERE hostname = ?`, "host-1")
and the other affected queries (the ones at the other noted locations) to use
WHERE node_key = ? (or osquery_host_id = ?) and pass the fixture's unique
node_key value; ensure any variables/helpers referencing the host (e.g., the
test fixture that provides "host-1") are updated to supply the unique
identifier.
In `@server/service/integration_core_test.go`:
- Around line 17031-17045: Add a positive boundary test for the Orbit debug
logging cap: call s.DoJSON with the PATCH to "/api/latest/fleet/config" using
agent_options.orbit.debug_logging_on_enroll_duration set to 86400 and assert
success (http.StatusOK) populating appConfigResponse; this complements the
existing negative tests for 86401 and -1 and verifies that the exact allowed
maximum is accepted. Ensure you use the same s.DoJSON helper and
appConfigResponse type as in the surrounding tests so the new assertion
integrates consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e271dbfd-7d95-4229-b197-20a3eb9dcf70
📒 Files selected for processing (21)
changes/43997-orbit-debug-logging-on-enrollorbit/changes/43997-orbit-debug-logging-on-enrollorbit/cmd/orbit/orbit.goorbit/pkg/update/debug_log_runner.goorbit/pkg/update/debug_log_runner_test.goorbit/pkg/update/flag_runner.goorbit/pkg/update/flag_runner_test.goserver/datastore/mysql/hosts.goserver/datastore/mysql/hosts_test.goserver/datastore/mysql/migrations/tables/20260512143542_AddOrbitDebugUntilToHosts.goserver/datastore/mysql/migrations/tables/20260512143542_AddOrbitDebugUntilToHosts_test.goserver/datastore/mysql/schema.sqlserver/fleet/agent_options.goserver/fleet/agent_options_test.goserver/fleet/datastore.goserver/fleet/hosts.goserver/fleet/orbit.goserver/mock/datastore_mock.goserver/service/integration_core_test.goserver/service/orbit.goserver/service/orbit_test.go
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
MagnusHJensen
left a comment
MagnusHJensen
left a comment
There was a problem hiding this comment.
DIdn't have the chance to test this on a device, but code looks good to me. So if you have tested it yourself, then I give my blessing to merge 😆
…ploy) Upstream PR fleetdm#45367 (aggregated commit bee5eda, 2026-05-14) added the orbit_debug_until column to the hosts table in MySQL schema.sql. The PG baseline is regenerated from a production pg_dump; that migration hasn't landed on prod yet (it lands when this very deploy rolls out), so the baseline lags by one column. Adding the entry to known_column_drift.txt with a deferred-regen comment, per the file header's prescribed workflow. Once the next aggregation runs after this deploy lands, the prod baseline will include orbit_debug_until and this allowlist entry can be removed. Without this entry, the validate-pg-compat CI gate fails check_column_drift on every aggregated build, blocking the build-ledo image publish that deploys this fix in the first place. Classic chicken-and-egg — break with a documented intentional drift.
…ploy) Upstream PR fleetdm#45367 (aggregated commit bee5eda, 2026-05-14) added the orbit_debug_until column to the hosts table in MySQL schema.sql. The PG baseline is regenerated from a production pg_dump; that migration hasn't landed on prod yet (it lands when this very deploy rolls out), so the baseline lags by one column. Adding the entry to known_column_drift.txt with a deferred-regen comment, per the file header's prescribed workflow. Once the next aggregation runs after this deploy lands, the prod baseline will include orbit_debug_until and this allowlist entry can be removed. Without this entry, the validate-pg-compat CI gate fails check_column_drift on every aggregated build, blocking the build-ledo image publish that deploys this fix in the first place. Classic chicken-and-egg — break with a documented intentional drift.
…ploy) Upstream PR fleetdm#45367 (aggregated commit bee5eda, 2026-05-14) added the orbit_debug_until column to the hosts table in MySQL schema.sql. The PG baseline is regenerated from a production pg_dump; that migration hasn't landed on prod yet (it lands when this very deploy rolls out), so the baseline lags by one column. Adding the entry to known_column_drift.txt with a deferred-regen comment, per the file header's prescribed workflow. Once the next aggregation runs after this deploy lands, the prod baseline will include orbit_debug_until and this allowlist entry can be removed. Without this entry, the validate-pg-compat CI gate fails check_column_drift on every aggregated build, blocking the build-ledo image publish that deploys this fix in the first place. Classic chicken-and-egg — break with a documented intentional drift.
|
I believe the changes at https://github.com/fleetdm/fleet/pull/45367/changes#diff-bc31589a4fd9aed0a59f1bd77a489e2f80edc722517d3290191665bca50e33d1R64-R71 resulted in a change of behavior for users who specified additional options in After the changes, an empty slice of flags from the Fleet server will get written, effectively overriding any values provided by the user. Before the change, it would return early if there were no flags coming from the Fleet server, keeping the user provided A user provided fleet/orbit/cmd/orbit/orbit.go Lines 1358 to 1365 in ed42b7b I'm hitting this issue as I'm trying to load extensions installed through an MSI (so not managed by TUF), so I need to provide |
Related issue: Resolves #43997
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Input data is properly validated,
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Timeouts are implemented and retries are limited to avoid infinite loops
If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes
Testing
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
Database migrations
COLLATE utf8mb4_unicode_ci).fleetd/orbit/Fleet Desktop
Summary by CodeRabbit
New Features
Chores
Tests