Skip to content

Feedback#3

Open
MagicRB wants to merge 6 commits intomainfrom
feedback
Open

Feedback#3
MagicRB wants to merge 6 commits intomainfrom
feedback

Conversation

@MagicRB
Copy link
Copy Markdown
Member

@MagicRB MagicRB commented Mar 17, 2026

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
Copy link
Copy Markdown
Member Author

MagicRB commented Mar 17, 2026

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

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

@MagicRB
Copy link
Copy Markdown
Member Author

MagicRB commented Mar 17, 2026

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
Copy link
Copy Markdown
Member Author

MagicRB commented Mar 17, 2026

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
Copy link
Copy Markdown
Member Author

MagicRB commented Mar 17, 2026

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