Skip to content

daemon: process ops when forcing untrusted clients - #16637

Open
faukah wants to merge 1 commit into
NixOS:masterfrom
faukah:faukah/push-ptvuqvokplvn
Open

faukah wants to merge 1 commit into
NixOS:masterfrom
faukah:faukah/push-ptvuqvokplvn

Conversation

@faukah

@faukah faukah commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Motivation

9796ebd ("Add --process-ops flag to nix-daemon") folded the condition for forwarding a --stdio connection into processOps without negating it. Since then, nix-daemon --stdio --force-untrusted forwards the connection to the next store, so the client is never treated as untrusted. A plain nix-daemon --stdio processes the ops itself instead of forwarding them, which made --process-ops a no-op.

We therefore need to only process the ops when the client is forced untrusted or --process-ops is passed.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

9796ebd ("Add `--process-ops` flag to `nix-daemon`") folded the
condition for forwarding a --stdio connection into processOps without
negating it. Since then, `nix-daemon --stdio --force-untrusted` forwards
the connection to the next store, so the client is never treated as
untrusted. A plain `nix-daemon --stdio` processes the ops itself
instead of forwarding them, which made --process-ops a no-op.

Only process the ops when the client is forced untrusted or
`--process-ops` is passed.
@faukah
faukah requested a review from edolstra as a code owner October 18, 2026 21:25
@github-actions github-actions Bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels Oct 18, 2026
Comment on lines +29 to +31
prog=$(readlink -e ./nix-daemon-untrusting.sh)
nix --store "ssh-ng://localhost?remote-program=$prog&remote-store=daemon" store info --json \
| jq -e '.trusted == false'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this test be in build-remote-.... where all existing trust override tests are?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should add store info calls there to make sure that the trust relation there holds.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah I guess it's fine to keep here, although we should probably just have a separate test for this testing all the various combinations.

This branch has not been deployed

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

Labels

new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants