Skip to content

Feedback#3

Open
MagicRB wants to merge 6 commits into
mainfrom
feedback
Open

Feedback#3
MagicRB wants to merge 6 commits into
mainfrom
feedback

Conversation

@MagicRB

@MagicRB MagicRB commented Mar 17, 2026

Copy link
Copy Markdown
Member

No description provided.

MagicRB added 3 commits March 17, 2026 13:17
Signed-off-by: Richard Brežák <richard@numtide.com>
Signed-off-by: Richard Brežák <richard@numtide.com>
Signed-off-by: Richard Brežák <richard@numtide.com>
@MagicRB

MagicRB commented Mar 17, 2026

Copy link
Copy Markdown
Member Author

let mut cmd = Command::new(&daemon_config.nix_daemon_path);

shouldn't we be opening the socket? that seems a bit more portable

@MagicRB

MagicRB commented Mar 17, 2026

Copy link
Copy Markdown
Member Author

let result = tokio::time::timeout(timeout, async {

if im reading this correctly, any single daemon connection can only be open for up to 5 minutes by default. That's not a lot and any number you could put in the timeout won't make much sense. Individual builds can take a long time. Also I'm not sure we need the timeout at all. WSs run over TCP which already handles system level timeouts and handling daemon protocol level timeouts would require parsing and reasoning about the data being passed back&forth imo.

MagicRB added 2 commits March 17, 2026 15:27
Signed-off-by: Richard Brežák <richard@numtide.com>
Signed-off-by: Richard Brežák <richard@numtide.com>
@MagicRB

MagicRB commented Mar 17, 2026

Copy link
Copy Markdown
Member Author

https://github.com/numtide/nix-relay/blob/feedback/src/auth.rs#L226

the documentation for this function says:

Whether to validate the aud field.

It will return an error if the aud field is not a member of the audience provided.

Validation only happens if aud claim is present in the token. Adding aud to required_spec_claims will make it required.

Defaults to true. Very insecure to turn this off. Only do this if you know what you are doing.

is this turned off for a reason? if so we should document that reason with a comment

Signed-off-by: Richard Brežák <richard@numtide.com>
@MagicRB

MagicRB commented Mar 17, 2026

Copy link
Copy Markdown
Member Author

Also, the CI didn't catch a failing check. 4b96629 introduced a bug which caused the nix-relay check to fail, yet the CI passed.

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.

1 participant