feat(builtins): implement journalctl#553
Draft
matt-dz wants to merge 35 commits into
Draft
Conversation
matt-dz
force-pushed
the
matthew.deguzman/journalctl-builtin
branch
from
de9b277 to
ad975ba
Compare
Collaborator
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0a48c66ff
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| func readMachineID(path string) (string, error) { | ||
| file, err := os.Open(path) |
There was a problem hiding this comment.
When a mounted target uses a stateless layout such as /host/etc/machine-id -> /run/machine-id, this plain os.Open follows the absolute symlink in the rshell process namespace instead of under the configured Root. In that case journal queries, disk usage, vacuuming, and rotation validation use the local machine ID and either fail to find the mounted host's journal directory or operate against the wrong ID, despite the Root target being documented as keeping derived paths on that mounted host.
Useful? React with 👍 / 👎.
matt-dz
changed the base branch from
main
to
matthew.deguzman/address-copilot-system-allowlist
Base automatically changed from
matthew.deguzman/address-copilot-system-allowlist
to
main
July 15, 2026 14:35
matt-dz
force-pushed
the
matthew.deguzman/journalctl-builtin
branch
from
c0a48c6 to
a73bf25
Compare
There was a problem hiding this comment.
Pull request overview
Implements a restricted journalctl builtin to query bounded unit/kernel logs and perform journald storage maintenance (disk usage, rotation, vacuum) under rshell’s default-deny security model, backed by a new trusted internal/systemd target+client layer and expanded systemd allowlist capabilities.
Changes:
- Added
journalctlbuiltin with a constrained flag surface (exact unit / kernel scope, bounded-n,--since,short|cat, plus--disk-usage,--rotate, and vacuum options). - Introduced trusted systemd targeting (
SystemdTargetConfig) and typed systemd service backends (internal/systemd) for reading journals, disk-usage, rotation (Varlink), and vacuuming archived journals. - Generalized and expanded the systemd allowlist model from service-only to generic systemd actions (
read|clean|reload|restart), and wired it through the interpreter, CLI, analyzer allowlists, docs, tests, and CI.
Reviewed changes
Copilot reviewed 50 out of 51 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/scenarios/cmd/journalctl/help/help.yaml | Scenario coverage for journalctl --help output. |
| tests/scenarios/cmd/journalctl/errors/vacuum_read_only.yaml | Scenario asserting vacuum requires remediation mode. |
| tests/scenarios/cmd/journalctl/errors/vacuum_denied.yaml | Scenario asserting vacuum requires clean allowlist grant. |
| tests/scenarios/cmd/journalctl/errors/rotate_read_only.yaml | Scenario asserting rotation requires remediation mode. |
| tests/scenarios/cmd/journalctl/errors/raw_match.yaml | Scenario rejecting arbitrary journal field matches. |
| tests/scenarios/cmd/journalctl/errors/missing_scope.yaml | Scenario enforcing exact unit or kernel scope requirement. |
| tests/scenarios/cmd/journalctl/errors/denied_unit.yaml | Scenario enforcing allowlist for unit log reads. |
| tests/scenarios/cmd/journalctl/errors/denied_storage.yaml | Scenario enforcing allowlist for disk usage reads. |
| SHELL_FEATURES.md | Documents new journalctl surface and updated systemd allowlist/target features. |
| README.md | Updates public docs for systemd capability policy, targets, and journalctl usage. |
| LICENSE-3rdparty.csv | Adds third-party attribution for go-systemd. |
| interp/systemd_target.go | Adds SystemdTargetConfig + runner option to configure trusted target. |
| interp/systemd_target_test.go | Tests for target defaults, copying, and invalid combinations. |
| interp/system_services.go | Extends allowlist actions to include clean and adds authorizeSystemd. |
| interp/system_services_test.go | Updates/extends allowlist authorization tests for new semantics. |
| interp/runner_exec.go | Plumbs AuthorizeSystemd and Systemd services into builtin call contexts. |
| interp/register_builtins.go | Registers the new journalctl builtin. |
| interp/journal_vacuum_integration_test.go | End-to-end integration test for vacuuming against a synthetic journal layout. |
| interp/api.go | Constructs the systemd client/services and defaults systemd target to local paths. |
| internal/systemd/target.go | Implements validation/resolution of local vs rooted vs explicit systemd targets. |
| internal/systemd/target_test.go | Unit tests for target resolution/validation. |
| internal/systemd/journal_varlink.go | Implements bounded Varlink call to journald rotate endpoint. |
| internal/systemd/journal_varlink_test.go | Tests for Varlink method use, protocol hardening, cancellation, endpoint checks. |
| internal/systemd/journal_vacuum_unsupported.go | Non-Unix stub for vacuum support. |
| internal/systemd/journal_vacuum_unix.go | Unix vacuum implementation using rooted directory pinning and strict archived-name filtering. |
| internal/systemd/journal_vacuum_unix_test.go | Vacuum behavior tests (age/size, dry-run, link handling, invalid layouts). |
| internal/systemd/journal_usage_unsupported.go | Non-Unix stub for disk-usage support. |
| internal/systemd/journal_usage_unix.go | Disk usage implementation based on allocated blocks. |
| internal/systemd/journal_usage_unix_test.go | Tests for usage counting and cancellation. |
| internal/systemd/journal_rotate_unsupported.go | Non-Linux stub for rotation support. |
| internal/systemd/journal_rotate_linux.go | Linux rotation implementation with timeout and machine-id/socket validation. |
| internal/systemd/journal_reader.go | Shared bounded query/match logic and field selection for journal reads. |
| internal/systemd/journal_reader_unsupported.go | Non-(linux+cgo) stub for journal reads with clear erroring. |
| internal/systemd/journal_reader_test.go | Tests for match construction, field selection, boot detection, query validation. |
| internal/systemd/journal_reader_linux_cgo.go | Linux+cgo reader using sdjournal from go-systemd. |
| internal/systemd/journal_files.go | Selects target journal files based on machine-id and strict directory scanning. |
| internal/systemd/journal_files_test.go | Tests for machine-id validation and file selection logic. |
| internal/systemd/journal_file_unix.go | Unix allocation metadata helper used by usage/vacuum. |
| internal/systemd/client.go | Systemd client wrapper that copies target config immutably. |
| go.mod | Adds github.com/coreos/go-systemd/v22. |
| go.sum | Adds checksums for go-systemd. |
| docs/RULES.md | Documents the trusted systemd target exception and journal mutation constraints. |
| cmd/rshell/main.go | Adds CLI flags for systemd targets and tightens service grant parsing. |
| cmd/rshell/main_test.go | Updates CLI tests for new systemd flags and allowlist parsing behavior. |
| builtins/systemd.go | Defines typed interfaces and data structures for systemd-aware builtins. |
| builtins/journalctl/journalctl.go | Implements restricted journalctl builtin, authorization, formatting, and maintenance ops. |
| builtins/journalctl/journalctl_test.go | Comprehensive unit tests for query building, escaping, exclusivity, rotation/vacuum flows. |
| builtins/builtins.go | Extends CallContext with AuthorizeSystemd and Systemd capabilities; adds systemd types. |
| analysis/symbols_builtins.go | Updates static analysis allowlists for the new builtin and CallContext capabilities. |
| .github/workflows/test.yml | Installs libsystemd-dev on CI runners where journal reading requires it. |
| .github/workflows/compliance.yml | Installs libsystemd-dev for compliance job coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+89
to
+97
| The only deletion exception is `JournalCleaner.VacuumJournal`. It is available | ||
| only through trusted systemd target configuration and a validated operator | ||
| vacuum policy. The backend pins each configured journal directory with `os.Root`, | ||
| checks directory identity across open, accepts only strict systemd archived-file | ||
| names, excludes symlinks and hardlinks, and revalidates file identity immediately | ||
| before rooted removal. Active, malformed, recently modified, and policy-retained | ||
| files must never be deleted. Every cleanup call has both file-count and byte | ||
| ceilings in addition to the shared `journal:storage/clean` authorization and | ||
| remediation-mode requirement. |
Comment on lines
+63
to
+70
| for _, candidate := range candidates { | ||
| if err := ctx.Err(); err != nil { | ||
| return result, vacuumPartialError(result, err) | ||
| } | ||
| expired := !request.Before.IsZero() && !candidate.modTime.After(request.Before) | ||
| overSize := request.MaxBytes > 0 && remainingBytes > request.MaxBytes | ||
| if !expired && !overSize { | ||
| break |
Comment on lines
+248
to
+268
| request := builtins.JournalVacuumRequest{} | ||
| if vacuumSet { | ||
| request.Now = callCtx.Now | ||
| request.DryRun = *options.dryRun | ||
| } | ||
| if sizeSet { | ||
| size, err := sizeparse.Parse(*options.vacuumSize) | ||
| if err != nil || size <= 0 { | ||
| callCtx.Errf("journalctl: invalid --vacuum-size value %q (must be greater than zero)\n", *options.vacuumSize) | ||
| return builtins.Result{Code: 1} | ||
| } | ||
| request.MaxBytes = uint64(size) | ||
| } | ||
| if timeSet { | ||
| age, err := time.ParseDuration(*options.vacuumTime) | ||
| if err != nil || age <= 0 { | ||
| callCtx.Errf("journalctl: invalid --vacuum-time value %q (use a positive Go duration such as 168h)\n", *options.vacuumTime) | ||
| return builtins.Result{Code: 1} | ||
| } | ||
| request.Before = callCtx.Now.Add(-age) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds a restricted
journalctlbuiltin for bounded log investigation and disk cleanup without executing host binaries.-u, current-boot kernel queries with-k,-b, bounded-n,--since, andshort/catoutput.--disk-usage, policy-bounded--vacuum-sizeand--vacuum-time,--dry-run, and synchronous--rotate.read,clean,reload, andrestartactions.systemctlintegration path.Authorization is scoped as follows:
unit:NAME/readfor every requested unitjournal:kernel/readjournal:storage/readjournal:storage/cleanplus remediation modeQueries are capped at 32 exact units and 1,000 entries. Vacuuming only considers strictly recognized archived journals and excludes active files, malformed names, symlinks, hardlinks, recently modified files, and policy-retained files.
systemctlis not included in this PR. The shared target and typed service model are intended to support it without exposing a generic D-Bus interface to builtins.Motivation
AI agents need to investigate unit and kernel logs and remediate journal disk usage while remaining inside rshell's default-deny security model.
Calling the host
journalctlbinary would expose a much broader command surface, including arbitrary matches, alternate files and directories, follow mode, cursors, machines, and namespaces. This implementation instead exposes a bounded subset with resource-level authorization and trusted operator controls around destructive cleanup.Testing
Passed locally:
make fmtgofmt -l .journalctl, internal systemd backends, authorization, CLI configuration, and static analysisgo test -race ./internal/systemd ./builtins/journalctl ./interpRSHELL_COMPLIANCE_TEST=1 go test ./tests/ -run TestCompliance -count=1The required Bash comparison command was attempted but skipped because the local Docker daemon was unavailable.
go test ./...reached all packages; its remaining failures were existing macOS sandbox restrictions around Unix socket creation, process sysctls, and socket enumeration. The Linux cgo/libsystemd reader path is left for CI validation.Checklist