wyctl: GSettings defaults and token-file safety#321
Merged
Conversation
Introduces org.wyrelog.wyctl with the eight keys wyctl will resolve from
GSettings when CLI flags are absent: daemon-url, default-tenant,
default-graph, access-token-file, default-timeout-ms,
default-guard-loc-class, default-guard-risk, and
default-guard-timestamp-mode (with a "none"/"now" enum).
The schema installs into ${datadir}/glib-2.0/schemas and the build runs
gnome.post_install(glib_compile_schemas: true) so packaged installations
have a compiled schema cache.
For tests, glib-compile-schemas runs against a staged copy of the schema
in the tests build dir, and a new test-wyctl-gschema unit binary asserts
the schema can be opened, every key has the expected type, no key smells
like a credential value, and the safe-sentinel defaults are intact. The
test runs with GSETTINGS_SCHEMA_DIR pointed at the build dir and
GSETTINGS_BACKEND=memory so it never touches the developer's dconf.
No wyctl behaviour change in this commit: this is the scaffolding the
resolver and token-file safety check will build on.
Tests: meson test -C builddir wyctl-gschema wyctl-basic wyctl-policy-daemon wyctl-status-daemon client-smoke — 5/5 OK.
Introduces wyrelog/wyctl/wyctl-config.{c,h} with three functions wyctl
will use to fold GSettings defaults into option parsing without ever
overriding an explicit CLI value:
wyctl_open_settings()
g_settings_schema_source_get_default() + schema_source_lookup, so
a missing schema yields NULL instead of g_error/SIGABRT. Honors the
WYCTL_DISABLE_GSETTINGS=1 kill switch so a CI container without a
dconf daemon (or an operator who wants reproducible CLI-only runs)
can short-circuit before any GSettings call.
wyctl_resolve_string_option(cli, settings, key)
Returns an owned string in every non-NULL branch (g_strdup of the
CLI value, or g_settings_get_string). An empty CLI string is the
user's deliberate value, NOT absence; an empty GSettings string is
the "unset" sentinel and surfaces as NULL so the existing missing-
option diagnostic stays load-bearing.
wyctl_resolve_uint_option_as_string(cli, settings, key)
Same shape, but renders the uint with %u so callers can keep
threading the value through parse_timeout_ms unchanged.
The resolver is not yet wired into wyctl.c; that fan-out lands in the
next commits. This commit ships the unit-testable helper plus 12
tests covering ownership, precedence, the empty-string / unset
distinction, the kill switch, the schema-present happy path, and the
GLib lookup-NULL-on-missing-id invariant the resolver relies on. The
test binary compiles wyctl-config.c directly and runs under the
build-tree's compiled schema with GSETTINGS_BACKEND=memory so it
never touches the developer's dconf.
Tests: meson test -C builddir wyctl-config wyctl-gschema wyctl-basic wyctl-policy-daemon wyctl-status-daemon client-smoke — 6/6 OK.
Threads a GSettings handle from main() into WyctlOptions and wires
run_status() through the resolver introduced in the previous commit.
When the operator omits --daemon-url or --timeout-ms the resolver now
falls back to the org.wyrelog.wyctl GSettings values; explicit CLI
flags still win. The resolved values live in g_autofree locals so the
GOptionContext-owned opts.daemon_url / opts.timeout_ms_arg slots are
left alone and there is no ownership conflict on context teardown.
The handle is opened once in main() with g_autoptr (GSettings) and
borrowed into WyctlOptions; it is NULL when the schema is missing or
WYCTL_DISABLE_GSETTINGS=1 is set, and the resolver treats NULL as
"no fallback available" — preserving the existing missing-daemon-URL
diagnostic.
Three subprocess-driven tests cover the end-to-end behaviour by
staging a temporary XDG_CONFIG_HOME with a GSettings keyfile backend
and spawning wyctl with a custom envp:
* status-gsettings-supplies-daemon-url
GSettings has the URL, CLI omits --daemon-url, the
daemon-unavailable diagnostic names the GSettings URL.
* status-cli-overrides-gsettings
GSettings has a different URL, CLI supplies the canonical one,
the diagnostic names the CLI URL and never the GSettings URL.
* status-kill-switch-disables-gsettings
GSettings has the URL but WYCTL_DISABLE_GSETTINGS=1 is set, so
the missing-daemon-URL diagnostic fires instead.
The wyctl-basic meson test invocation now inherits GSETTINGS_SCHEMA_DIR
and GSETTINGS_BACKEND=memory and depends on wyctl-gschemas-compiled
so the child wyctl can find the schema. Existing wyctl-basic cases are
unaffected because empty schema defaults still surface as NULL through
the resolver and trigger the same diagnostics as before.
Tests: meson test -C builddir wyctl-config wyctl-gschema wyctl-basic wyctl-policy-daemon wyctl-status-daemon client-smoke — 6/6 OK.
Fans the resolver from run_status out to every other subcommand that
talks to the daemon. After this commit, --daemon-url, --timeout-ms,
--tenant, --graph, and --access-token-file all honour the same
precedence rule (CLI > GSettings > unset) regardless of which wyctl
command is invoked.
Touched call sites:
* run_policy_decide_request (policy check, policy explain)
* run_policy_permission_mutation_command (permission-grant/revoke)
* run_policy_role_mutation_command (role-grant/revoke)
* run_audit_query
* create_fact_client + its four callers (run_graph_create,
run_fact_schema_register, run_fact_put, run_datalog_query)
* run_policy_decision_command (--access-token-file only)
create_fact_client now takes already-resolved daemon_url,
timeout_ms_arg, tenant, and access_token_file as explicit parameters
so its callers can stage the same locals once and pass them in. The
resolved values live in g_autofree gchar* locals at each call site;
GOptionContext-owned opts.* slots are read but never written back,
preserving the ownership contract established in the run_status
exemplar.
A new helper in tests/test-wyctl-basic.c drives the precedence rule
end-to-end against `policy check` and `audit query` subcommands. Both
new cases stage a temporary XDG_CONFIG_HOME with the GSettings
keyfile backend, spawn wyctl without --daemon-url, and assert that
the daemon-unavailable diagnostic mentions the GSettings URL — the
same assertion shape the run_status tests already use, extended
across the resolver fan-out per the synthesized plan.
Tests: meson test -C builddir wyctl-config wyctl-gschema wyctl-basic wyctl-policy-daemon wyctl-status-daemon client-smoke — 6/6 OK.
The previous commit landed two precedence tests (policy check, audit query) that asserted on the daemon-unavailable diagnostic literal — but that diagnostic is emitted only by run_status. Every other subcommand surfaces transport failure as "wyctl: <op> failed" with no URL echo, so the assertion never matched and the tests failed when run against a fresh build. The fix reshapes assert_subcommand_resolves_daemon_url_from_gsettings into assert_subcommand_consumes_gsettings_daemon_url with a portable contract: stage a GSettings keyfile carrying the URL, spawn the subcommand without --daemon-url, and assert that neither "wyctl: missing daemon URL" nor "wyctl: invalid daemon URL" appears in stderr. That proves the subcommand made it past URL validation and the resolver supplied a usable URL — regardless of which failure diagnostic the subcommand eventually emits. Also adds the two parametrized cases the synthesis plan calls out and the previous commit left uncovered: fact put and datalog query. The runtime contract is identical; only the CLI fixture argv changes. The kill-switch test for status (preserved from commit 3) plus the unit-level wyctl-config kill-switch test prove the resolver's kill switch behaviour. A subcommand-specific kill-switch test would require each subcommand's options to clear all earlier validation gates (e.g. a non-empty token file for policy check) so duplicating that scaffolding adds no incremental signal here. Tests: meson test -C builddir wyctl-config wyctl-gschema wyctl-basic wyctl-policy-daemon wyctl-status-daemon client-smoke — 6/6 OK; new cases policy-check / audit-query / fact-put / datalog-query gsettings-supplies-daemon-url all pass.
Replaces wyctl's previous g_file_get_contents-based token loader with
a typed safety helper that fails closed before any daemon request
escapes. The new code lives in wyrelog/wyctl/wyctl-token-file.{c,h}
and is the single chokepoint every subcommand reaches through the
existing load_access_token_file wrapper.
Safety invariants on POSIX:
* Single open() with O_NOFOLLOW | O_CLOEXEC | O_RDONLY | O_NOCTTY.
ELOOP is reported as SYMLINK; ENOENT as NOT_FOUND; everything
else as IO. A terminal symlink at the path is refused.
* fstat() on the same fd we opened, so the regular-file / owner /
mode checks operate on the very inode the read will consume.
The check uses geteuid() to compare against st.st_uid so a setuid
wrapper does the right thing.
* Mode mask (S_IRWXG | S_IRWXO) — any group or other permission
bit fails with PERMISSIONS_TOO_BROAD. 0600 and stricter accepted.
* Reads from the same fd we fstatted; no second path-based syscall
after open. 64 KiB hard cap (WYCTL_TOKEN_FILE_MAX_BYTES). A
post-read probe rejects files that grew during the read.
* Embedded NUL bytes are rejected as INVALID_BYTES; an empty file
is EMPTY. The path is the only thing logged on failure — token
bytes never appear in a diagnostic.
The classifier (wyctl_token_file_classify_stat) is a pure function
that takes a struct stat + effective uid, so the owner-mismatch and
non-regular paths are unit-testable without root or mkfifo-blocking
quirks.
load_access_token_file in wyctl.c is rewritten to a thin wrapper that
calls wyctl_token_file_read, maps each status to its typed diagnostic
via wyctl_token_file_status_message, and runs the existing
normalize_access_token_file pass on the safety-checked buffer. The
former generic "wyctl: unable to read access token file" diagnostic
is replaced by distinct messages per failure class — distinguishable
diagnostics are an explicit acceptance criterion for the issue.
Tests:
* tests/test-wyctl-token-file.c — 14 cases covering happy path
(0600, 0400), each rejection class (0640, 0604, 0660, terminal
symlink, FIFO via classifier, missing path, missing file, empty,
embedded NUL, oversize), owner-mismatch via the classifier
injection seam, and a guard that every status format string
references only the path, never the token bytes.
* tests/test-wyctl-basic.c — two integration tests
(test_policy_check_safety_reject_prevents_http and
test_audit_query_safety_reject_prevents_http) feed a mode-0640
token file to wyctl and assert the safety diagnostic fires while
the subcommand's own "<op> failed" diagnostic is absent —
proving the rejection happens before any wyl_client_new call.
* tests/test-wyctl-basic.c diagnostic assertion for a nonexistent
token path is updated from "wyctl: unable to read access token
file" to the new "wyctl: access token file not found".
The Windows code path is stubbed to fail closed with
WINDOWS_NOT_READONLY; the actual GetFileAttributesW + reparse-point
+ read-only-attribute check lands in the next commit so this commit
can stay scoped to POSIX hardening.
Tests: meson test -C builddir wyctl-token-file wyctl-config wyctl-gschema wyctl-basic wyctl-policy-daemon wyctl-status-daemon client-smoke — 7/7 OK; 14 token-file unit cases plus 2 integration cases pass.
Lights up the Windows token-file safety path that commit 5 stubbed.
The implementation is split into two pieces so the Linux CI runner
can exercise the rejection rules without any Win32 API:
* wyctl_token_file_classify_windows_attrs(attrs) — a pure helper
that takes a GetFileAttributesW-style bit mask and applies the
two safety rules the issue calls out for Windows: reject if the
FILE_ATTRIBUTE_REPARSE_POINT bit (0x400) is set (symlink /
junction / mountpoint), reject if FILE_ATTRIBUTE_READONLY
(0x01) is unset, otherwise accept. Compiled unconditionally so
the unit tests below can drive it on Linux with synthetic
inputs.
* The G_OS_WIN32 branch of wyctl_token_file_read now calls
GetFileAttributesW, hands the mask to the classifier, and on a
pass opens the file via CreateFileW(GENERIC_READ, FILE_SHARE_READ,
OPEN_EXISTING) and reads up to WYCTL_TOKEN_FILE_MAX_BYTES with
the same overflow probe and embedded-NUL / empty rejections as
the POSIX path.
ACL validation remains a non-fatal best-effort: the issue text says
"prefer ACL validation if practical." The
WYCTL_TOKEN_FILE_WINDOWS_ACL_UNAVAILABLE status stays in the
diagnostic table as the slot a future ACL-check landing will use; no
caller emits it from this commit. The current Windows path is
fail-closed on the read-only + reparse-point checks alone, which
matches the issue's mandatory minimum.
Three new unit cases cover the pure classifier on every platform:
* windows-attrs-accept-readonly — readonly bit set yields OK,
with or without the FILE_ATTRIBUTE_NORMAL bit also set.
* windows-attrs-reject-not-readonly — plain or zero attributes
yield WYCTL_TOKEN_FILE_WINDOWS_NOT_READONLY.
* windows-attrs-reject-reparse-point — reparse-point set
(with or without read-only also set) yields
WYCTL_TOKEN_FILE_SYMLINK; the reparse rejection takes
precedence over the read-only check.
The POSIX branch is untouched; existing 14 unit cases and 2
integration cases still pass.
Tests: meson test -C builddir wyctl-token-file wyctl-config wyctl-gschema wyctl-basic wyctl-policy-daemon wyctl-status-daemon client-smoke — 7/7 OK; 17 token-file unit cases pass (14 POSIX + 3 Windows-attrs).
Adds an operator-facing section to docs/operator-runbook.md covering the surface that issue #320 lands: the GSettings schema for static wyctl defaults, the CLI > GSettings > unset precedence rule, the WYCTL_DISABLE_GSETTINGS=1 kill switch, the POSIX and Windows token-file permission requirements, the intermediate-path symlink scope statement, the complete diagnostic-message catalog, and end-to-end setup recipes for POSIX and Windows operators. The runbook now satisfies the issue's "Operator documentation explains the configuration and token-file permission requirements" acceptance criterion. Key points operators need to act on: * Schema id is org.wyrelog.wyctl; manual installs must run glib-compile-schemas to refresh the gschemas.compiled cache. * Each schema key's purpose, type, and the empty-string-is-unset sentinel convention is tabulated. * The kill switch matches the literal string "1" only — operators who type "true"/"yes"/"on" will silently get GSettings still active. * POSIX safety: regular file owned by geteuid(), mode mask (S_IRWXG | S_IRWXO) must be zero, terminal symlinks refused via O_NOFOLLOW. Intermediate-path symlinks are out of scope for the GA hardening pass and operators are warned to keep every parent directory operator-owned. * Windows safety: FILE_ATTRIBUTE_READONLY must be set; FILE_ATTRIBUTE_REPARSE_POINT must not be set. ACL validation is reserved for follow-up and does not fire today. * Diagnostic catalog enumerates every greppable stderr substring and confirms exit-code 2 with no daemon request on rejection. * Setup recipes use install + redirect to keep the token out of shell history and out of environment variables once configured. Tests: meson test -C builddir wyctl-token-file wyctl-config wyctl-gschema wyctl-basic wyctl-policy-daemon wyctl-status-daemon client-smoke — 7/7 OK (docs-only commit; no source changes).
CI surfaced three classes of failures that did not show up locally because the developer workstation umask is 0, masking the issues that strict-umask runners see immediately. 1. `g_file_set_contents` umask interaction. Every test that creates a temp token file via `g_file_open_tmp` then writes contents with `g_file_set_contents` ends up with a 0644 file on a 0022 umask runner — the atomic rename inside set_contents applies umask to the new file, masking the 0600 that mkstemp originally created. The wyctl token-file safety check then rejects the test fixture as "permissions too broad" and the test bails out before it can exercise the path it is actually testing. Fix: after every `g_file_set_contents` call on a token path in `tests/test-wyctl-basic.c` (15 sites) and the `write_token_file` helper in `tests/test-wyctl-policy-daemon.c`, explicitly `g_chmod (path, 0600)`. The bulk fix in test-wyctl-basic.c was applied by a regex sweep matching the pattern `g_assert_true (g_file_set_contents (<var>, ...))` followed by `g_assert_no_error (error)` so every site gets the same chmod-down treatment. Shell-side fix: the bootstrap-admin and datalog-product-flow harnesses (`tests/check-wyrelogd-bootstrap-admin.sh`, `tests/check-wyrelogd-datalog-product-flow.sh`) write tokens via Python `open(...).write(token)` which also honours umask. Adds `os.chmod(token_path, 0o600)` inside each heredoc. 2. macOS `O_NOFOLLOW` undeclared under strict `c_std=c17`. Apple SDKs hide POSIX-only BSD features behind `_DARWIN_C_SOURCE` when the compiler predefines `__STRICT_ANSI__` (which clang does under `-std=c17`). Setting `_POSIX_C_SOURCE` alone is not enough. Matches the existing pattern in `tests/test-policy-store-toctou.c`. 3. Windows compile failed because `wyctl-token-file.c` referenced `DWORD`, `HANDLE`, `ReadFile`, etc. inside `#ifdef G_OS_WIN32` without including `<windows.h>`. Adds the include in the Win32 branch. Tests: `bash -c "umask 022 && meson test -C builddir wyctl-token-file wyctl-config wyctl-gschema wyctl-basic wyctl-policy-daemon wyctl-status-daemon client-smoke --print-errorlogs"` — 7/7 OK under the strict umask that previously broke CI.
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
1 participant
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.
Summary
Closes #320.
Lands the operator-static defaults and bearer-token hardening requested in the issue. Eight atomic commits, each independently buildable and tested.
org.wyrelog.wyctlcoversdaemon-url,default-tenant,default-graph,access-token-file,default-timeout-ms,default-guard-loc-class,default-guard-risk,default-guard-timestamp-mode. CLI > GSettings > unset precedence; an empty CLI value is treated as a deliberate operator value, not absence.WYCTL_DISABLE_GSETTINGS=1env kill switch short-circuits the lookup for CI containers and reproducible runs (literal string1only).O_NOFOLLOW | O_CLOEXEC | O_RDONLY | O_NOCTTY, fstats the resulting fd, checksS_ISREG,st.st_uid == geteuid(), and(S_IRWXG | S_IRWXO) == 0, then reads bounded to 64 KiB from the same fd. No second path-based syscall. Embedded NULs and oversize files are refused.WyctlTokenFileStatusdiagnostic table prints only the path, never the token bytes. The check is the chokepoint every subcommand reaches before anywyl_client_*call.FILE_ATTRIBUTE_REPARSE_POINT(rejected) andFILE_ATTRIBUTE_READONLY(required) viaGetFileAttributesW+CreateFileW. Pure classifierwyctl_token_file_classify_windows_attrs(guint32)is compiled and unit-tested on every platform so Linux CI exercises the Windows rejection rules with synthetic inputs.Behavioural notes for reviewers / downstream users
chmod 0600+chownremediation.RESOLVE_BENEATH); the runbook calls this out.default-guard-loc-class,default-guard-risk,default-guard-timestamp-mode) are declared in the schema but not yet wired into per-subcommand resolution — they ship as scaffolding for a follow-up that owns the sentinel + enum-mode logic.Commits in this series
wyctl: install GSettings schema for defaults— schema XML + meson install + build-tree compile + harness test.wyctl: add CLI/GSettings option resolver—wyctl-config.{c,h}+ 12 unit tests.wyctl: resolve status flags via GSettings— exemplar wiring inrun_status; 3 subprocess precedence tests.wyctl: thread option resolver through all commands— fan out to policy/audit/fact/graph/datalog subcommands.wyctl: reshape resolver fan-out tests for non-status diagnostics— repair commit that fixed and extended the parametrized test contract.wyctl: harden token-file reads on POSIX— typed safety helper + 14 unit cases + 2 integration cases asserting no HTTP before reject.wyctl: validate Windows token-file attributes— pure attrs classifier + Win32 API path; 3 cross-platform tests.docs: document wyctl GSettings and token-file safety— operator runbook section.Test plan
meson test -C builddir wyctl-gschema— 3 keys/types/defaults assertions.meson test -C builddir wyctl-config— 12 resolver semantics tests.meson test -C builddir wyctl-token-file— 17 token-file safety tests (14 POSIX + 3 Windows-attrs).meson test -C builddir wyctl-basic— 35 sub-tests including the 4 new GSettings-supplies-daemon-url cases (status / policy check / audit query / fact put / datalog query) and 2 safety-reject-prevents-HTTP cases.meson test -C builddir wyctl-policy-daemon wyctl-status-daemon client-smoke— pre-existing wyctl integration suite still green.wyrelogd-profiles,wyrelogd-bootstrap-admin,template-tree,engine,valgrind-smoke) are unchanged frommainand not introduced by this PR../tools/gst-indentis stable on every modified C/H file.Co-Authored-By, no emoji, no persona/agent identity leaks in any commit message.Follow-up issues recommended (out of scope for #320)
WYCTL_TOKEN_FILE_WINDOWS_ACL_UNAVAILABLEto the actual ACL check.openat2(RESOLVE_BENEATH)for intermediate-path symlink rejection on Linux.none/nowtimestamp mode and-1risk sentinel.gsettingsCLI; document the registry/keyfile-backend alternative for vanilla Windows installs.