Skip to content

fix: stop routing OS users into custom host classes#573

Open
Scott-Guest wants to merge 1 commit into
denful:mainfrom
Scott-Guest:fix/user-to-host
Open

fix: stop routing OS users into custom host classes#573
Scott-Guest wants to merge 1 commit into
denful:mainfrom
Scott-Guest:fix/user-to-host

Conversation

@Scott-Guest

Copy link
Copy Markdown

Bug

den.batteries.os-user is documented as forwarding the user class into OS-level users.users.<name> on NixOS/nix-Darwin hosts. However, the policy actually routes unconditionally into host.class, so custom host classes receive a users.users.<name> entry, even when no OS user module exists there.

Changes

  • Guard den.policies.user-to-host so it only emits routes for host.class == "nixos" or "darwin".
  • Add a regression test covering the custom host class case

AI Assistance

Codex with GPT-5.5 xhigh was used to identify the issue, prepare this patch, draft the commit message, and draft this PR description. The work was guided, reviewed, and accepted by myself.

I, Scott Guest, explicitly acknowledge the following:

  • My contribution is AI generated and I mention which models/mcp-servers/tools were used.
  • I recognize AI generated contribution as my own and am myself legally accountable for it.
  • My contribution aligns with Den LICENSE and does not violate any other projects licenses (e.g. GPL)
  • I will truthfully answer and provide any requested details on how contribution was generated.

@Scott-Guest Scott-Guest requested review from sini and vic as code owners May 29, 2026 04:33
Comment thread modules/aspects/batteries/os-user.nix Outdated
})
];
lib.optional
(builtins.elem host.class [

@drupol drupol May 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might perhaps replace this with when attribute in the policy definition. To be checked.

https://den.denful.dev/explanation/policy-activation/#denlibpolicywhen-predicate-policy

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and I verified that it's equivalent. Amended.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can add the when attribute inside the policy you want to introduce, not outside and before it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, ignore this comment. That said, it might be an idea for @sini ?

E.g., we could do something like:

den.policies.user-to-host =
        { user, host, ... }:
        [
          (den.lib.policy.route {
            fromClass = "user";
            intoClass = host.class;
            path = [
              "users"
              "users"
              user.userName
            ];
            adaptArgs = args: args // { osConfig = args.config; };
            when = { user, host, ... }:
                builtins.elem host.class [ 
                  "nixos"
                  "darwin"
                ];
          })
        ]

Guard `den.policies.user-to-host` so it only emits routes for
`host.class == "nixos"` or `"darwin"`. This avoids introducing
`users.users.<name>` entries on custom host classes that may not have
an OS-level `users` module.
@sini

sini commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Can you describe the custom host usage pattern more? You can also just add this policy to den.defaults.excludes list to disable it.

den.lib.policy.when
(
{ user, host, ... }:
builtins.elem host.class [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might break with wsl or some microvm configs; I would like to understand the actual problem that made this an issue before reviewing the solution to it

@Scott-Guest

Scott-Guest commented May 30, 2026

Copy link
Copy Markdown
Author

Can you describe the custom host usage pattern more? You can also just add this policy to den.defaults.excludes list to disable it.

"custom" is a bit of a misnomer for my exact use case. I have host.class = "systemManager", and System Manager does actually support users.users. Disabling the policy is fine as a workaround.

But regardless, I think the issue pointed at by this PR is real:

  • os-user has documentation (here and here) implying it's for NixOS/nix-Darwin hosts.
  • user-to-host routes into whatever host.class is, so it can emit users.users.<name> even if host.class is some custom class that does not support users.users.
  • From what I can gather, other batteries guard config to the specific supported classes, avoiding this sort of issue.

Looking at this deeper, my actual usage is running into a related issue that's closer to a UX bug, which this PR doesn't address. Say I have:

{
  den.hosts.x86_64-linux.workstation = {
    class = "systemManager";
    users.tux.classes = [ ]
  };
}

This config does not provide any explicit den.aspects.tux.user = { ... }, and it opts out of the defaulted classes = [ "user" ], so I would expect to only get the Den-level user entity. I would not expect an OS-level users.users.tux entry.

Somewhat surprisingly though, System Manager fails with assertions like:

Failed assertions:
- Exactly one of users.users.tux.isSystemUser and users.users.tux.isNormalUser must be set.
- users.users.tux.group is unset. This used to default to nogroup, but this is unsafe.

It turns out that, as mentioned in this comment, user-to-host can create an empty placeholder OS-level users.users.tux = { }; even when there is no real user class content to forward. This is useful for integrated Home Manager on NixOS/nix-Darwin because HM reads config.users.users.<name>.name / .home from the parent OS config.

This is surprising: user-to-host can silently add an OS-level users.users.<name> option (and an empty/incomplete one at that), solely from the existence of a Den user entity, even outside the HM-integrated-with-NixOS/nix-Darwin case where it may be necessary.

Comment on lines +44 to +66
den.lib.policy.when
(
{ user, host, ... }:
builtins.elem host.class [
"nixos"
"darwin"
]
)
(
{ user, host, ... }:
[
(den.lib.policy.route {
fromClass = "user";
intoClass = host.class;
path = [
"users"
"users"
user.userName
];
adaptArgs = args: args // { osConfig = args.config; };
})
]
);

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants