Skip to content

fix(libstore): improve legacy ssh addToStore diagnostics#15503

Open
znaniye wants to merge 2 commits intoNixOS:masterfrom
znaniye:fix/add-to-store-diagnostics
Open

fix(libstore): improve legacy ssh addToStore diagnostics#15503
znaniye wants to merge 2 commits intoNixOS:masterfrom
znaniye:fix/add-to-store-diagnostics

Conversation

@znaniye
Copy link
Copy Markdown
Member

@znaniye znaniye commented Mar 17, 2026

Motivation

nix copy --to ssh://... can fail with a misleading low-level deserialisation error when the remote side returns a plain-text rejection instead of the expected binary status reply.

In this situation, users see a transport/protocol-looking failure even though the real issue is a policy rejection on the remote side.

Context

In a real-world flow, copying an unsigned store path to a remote machine as a non-trusted user produced:

error (ignored): error: interrupted by the user
error: serialised integer 7142773272180060773 is too large for type 'j'

This is hard to act on because it hides the underlying remote reason.

The actual remote-side cause was a trust/signature policy failure (path lacks a signature by a trusted key), but the client attempted to parse that text as a binary integer.

User-Visible Result

With this change, the same class of failure now reports a directly actionable error, for example:

error (ignored): failed to add path '/nix/store/<hash>-<name>' to remote host '<remote-host>': remote returned text where a binary reply was expected: 'error: cannot add path '/nix/store/<hash>-<name>' because it lacks a signature by a trusted key'
error: failed to add path '/nix/store/<hash>-<name>' to remote host '<remote-host>': remote returned text where a binary reply was expected: 'error: cannot add path '/nix/store/<hash>-<name>' because it lacks a signature by a trusted key'

This keeps protocol handling behavior intact while making failures diagnosable at the right abstraction level.

@znaniye znaniye requested a review from Ericson2314 as a code owner March 17, 2026 14:01
@github-actions github-actions Bot added the store Issues and pull requests concerning the Nix store label Mar 17, 2026
Capture non-binary replies from remote nix-store --serve so protocol mismatches report actionable context instead of opaque serialisation failures.
@znaniye znaniye force-pushed the fix/add-to-store-diagnostics branch from 41ebf91 to 2db2f06 Compare March 17, 2026 14:07
@znaniye znaniye requested a review from edolstra as a code owner March 17, 2026 14:43
@github-actions github-actions Bot added the with-tests Issues related to testing. PRs with tests have some priority label Mar 17, 2026
@znaniye znaniye marked this pull request as draft March 17, 2026 14:57
@znaniye znaniye force-pushed the fix/add-to-store-diagnostics branch from 7ded4e5 to d44844e Compare March 17, 2026 15:13
@znaniye znaniye marked this pull request as ready for review March 17, 2026 15:13
@znaniye znaniye marked this pull request as draft March 17, 2026 17:31
Cover legacy ssh addToStore failures that return textual remote rejections, asserting both the remote payload and the actionable error diagnostics exposed to users.
@znaniye znaniye force-pushed the fix/add-to-store-diagnostics branch from d44844e to 0dddef1 Compare March 17, 2026 21:50
@znaniye znaniye marked this pull request as ready for review March 17, 2026 22:37
@znaniye
Copy link
Copy Markdown
Member Author

znaniye commented Mar 17, 2026

legacy-ssh-store-no-write.sh is an intentional test fixture.

It strips --write from nix-store --serve --write ..., forcing the remote to return a real textual rejection (importing paths is not allowed) instead of a binary reply. This deterministically exercises the legacy-ssh-store.cc fix for “remote returned text where a binary reply was expected” and prevents regression to low-level serialization errors. Using plain remote-program=nix-store does not trigger this path, and a broken/missing program fails for unrelated reasons.

If you guys know a better way to trigger this binary-vs-text reply path in tests, I’m happy to switch. Suggestions welcome, and thanks in advance.

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

Labels

store Issues and pull requests concerning the Nix store 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.

1 participant