From a9d266c3bcb9c60804d6dae980bc888762588d45 Mon Sep 17 00:00:00 2001 From: Jason Bowman Date: Mon, 8 Jun 2026 09:30:50 -0700 Subject: [PATCH] fix(route): guarded empty-path route skips cleanly; honor intoPath alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A guarded simple route with an empty path crashed with "the option `_file' does not exist" when the guard was false: the raw collector module (`{ key; _file; imports }`) was merged under `config` — mis-reading module metadata as option definitions — and gated with `mkIf`, which still requires the target option to exist even when the condition is false. `guardModule` now mirrors the forward path: it gates with `lib.optionalAttrs` (dropping the subtree entirely when false) and recurses into structural imports, leaving module metadata at module level. Separately, `policy.route` silently dropped the `intoPath` key (only `path` was read), landing content at the class root instead of nesting. `route` now accepts `intoPath` as the public alias for `path` — pairing with `intoClass` and `fromClass` to match the forward API — and throws if both are given. Regression tests in deadbugs cover both: guard-false skips cleanly, and `intoPath` nests at the target path. --- nix/lib/aspects/fx/route/wrap.nix | 28 +++++- nix/lib/policy-effects.nix | 27 ++++-- .../route-intopath-conditional-leak.nix | 95 +++++++++++++++++++ 3 files changed, 138 insertions(+), 12 deletions(-) create mode 100644 templates/ci/modules/deadbugs/route-intopath-conditional-leak.nix diff --git a/nix/lib/aspects/fx/route/wrap.nix b/nix/lib/aspects/fx/route/wrap.nix index ec7ff1835..a57bed952 100644 --- a/nix/lib/aspects/fx/route/wrap.nix +++ b/nix/lib/aspects/fx/route/wrap.nix @@ -101,18 +101,36 @@ let nestPlain path mod; # Wrap a module with a conditional guard. + # + # A bool guard gates content with `lib.optionalAttrs`, not `lib.mkIf`, to + # match the forward path (forward.nix guardFn): a false guard must contribute + # *nothing* — `mkIf false` still requires the target option to exist, so an + # empty-path route into an undeclared option would fail option type-checking + # even when skipped. `optionalAttrs false` drops the subtree entirely. + # + # A structural module (carries `imports`/`_file`/`key` module metadata but no + # flat `config`) cannot be merged under `config` — its module-level keys would + # be mis-read as option definitions and fail (e.g. "the option `_file' does + # not exist"). This is the empty-path case, where the source is the raw + # collector module. Recurse into its `imports`, gating each leaf's config and + # leaving module metadata at module level. guardModule = guard: mod: if guard == null then mod else - args: let - inner = if builtins.isFunction mod then mod args else mod; + guardOne = + node: args: + let + inner = if builtins.isFunction node then node args else node; + in + if inner ? imports && !(inner ? config) then + { imports = map guardOne inner.imports; } // builtins.removeAttrs inner [ "imports" ] + else + { config = lib.optionalAttrs (guard args) (inner.config or inner); }; in - { - config = lib.mkIf (guard args) (inner.config or inner); - }; + guardOne mod; # Apply the adapt → nest → guard pipeline to a list of modules. # When adaptArgs is non-null (nestWithAdaptArgs path), all modules are diff --git a/nix/lib/policy-effects.nix b/nix/lib/policy-effects.nix index 2d46e80fd..023d36cde 100644 --- a/nix/lib/policy-effects.nix +++ b/nix/lib/policy-effects.nix @@ -99,13 +99,26 @@ in # Route class or quirk content from one scope partition into a target class. # Tier 1 delivery — replaces den.batteries.forward for the common case. - route = spec: { - __policyEffect = "route"; - value = { - path = [ ]; - } - // spec; - }; + # + # `intoPath` is the public target-path name — it pairs with `intoClass` and + # `fromClass`, matching the forward API. `path` is kept as a back-compat + # alias; both normalize to the internal `path` key the route handler reads. + # Passing an unsupported key (e.g. `intoPath` before this aliasing existed) + # used to be silently dropped, landing content at the class root. + route = + spec: + let + path = spec.intoPath or spec.path or [ ]; + in + if (spec ? intoPath) && (spec ? path) then + throw "den: policy.route: pass either `intoPath` or `path`, not both" + else + { + __policyEffect = "route"; + value = builtins.removeAttrs spec [ "intoPath" ] // { + inherit path; + }; + }; # Request post-pipeline instantiation of an entity's class content. # The entity carries instantiate, intoAttr, mainModule metadata. diff --git a/templates/ci/modules/deadbugs/route-intopath-conditional-leak.nix b/templates/ci/modules/deadbugs/route-intopath-conditional-leak.nix new file mode 100644 index 000000000..6ecb08904 --- /dev/null +++ b/templates/ci/modules/deadbugs/route-intopath-conditional-leak.nix @@ -0,0 +1,95 @@ +# Two related bugs in simple `policy.route`, surfaced by a guarded route that +# uses `intoPath` (the forward-style target-path name). +# +# Bug 1 (route/wrap.nix `guardModule`): a guarded route whose guard is false +# crashed with +# error: The option `_file' does not exist. Definition values: +# { _type = "if"; condition = false; content = "foo@igloo"; } +# because the raw collector module `{ key; _file; imports }` was merged under +# `config` (metadata mis-read as options) and gated with `mkIf` (which still +# requires the target option to exist). Fixed by gating with `optionalAttrs` +# (matching the forward path) and recursing into structural imports. +# +# Bug 2 (policy-effects.nix `route`): `intoPath` was silently dropped — only +# `path` was read — so content landed at the class root instead of nesting. +# Fixed by accepting `intoPath` as the public alias for `path`. +{ denTest, lib, ... }: +let + mkBox = + name: + { lib, ... }: + { + options.${name} = lib.mkOption { + type = lib.types.submoduleWith { + modules = [ + { + options.items = lib.mkOption { + type = lib.types.listOf lib.types.str; + default = [ ]; + }; + } + ]; + }; + default = { }; + }; + }; +in +{ + flake.tests.route-intopath-conditional-leak = { + + # Bug 1: guard false (nixos has no `foo` option) → route contributes + # nothing, cleanly, instead of crashing. + test-guarded-route-skips-cleanly = denTest ( + { den, igloo, ... }: + { + den.hosts.x86_64-linux.igloo = { }; + den.classes.foo.description = "foo class"; + + den.policies.foo-to-host = + { host, ... }: + den.lib.policy.route { + fromClass = "foo"; + intoClass = host.class; + intoPath = [ "foo" ]; + guard = { options, ... }: options ? foo; + }; + + den.schema.host.includes = [ den.policies.foo-to-host ]; + + den.aspects.igloo.foo.bar = "baz"; + + expr = igloo ? foo; + expected = false; + } + ); + + # Bug 2: `intoPath` nests content at the target path (was silently dropped, + # landing at the class root). + test-intopath-nests = denTest ( + { den, igloo, ... }: + { + den.hosts.x86_64-linux.igloo.users.tux = { }; + den.classes.src.description = "src class"; + + den.policies.src-to-host = + { host, ... }: + den.lib.policy.route { + fromClass = "src"; + intoClass = host.class; + intoPath = [ "wrapper" ]; + }; + + den.default.includes = [ den.policies.src-to-host ]; + + den.aspects.igloo = { + nixos.imports = [ (mkBox "wrapper") ]; + src.items = [ "routed" ]; + }; + + expr = igloo.wrapper.items; + expected = [ "routed" ]; + } + ); + + }; +}