Skip to content

allowed-uris: match whole schemes without slashes#9547

Merged
edolstra merged 9 commits intoNixOS:masterfrom
hercules-ci:allowed-scheme-without-slash
Dec 13, 2023
Merged

allowed-uris: match whole schemes without slashes#9547
edolstra merged 9 commits intoNixOS:masterfrom
hercules-ci:allowed-scheme-without-slash

Conversation

@roberth
Copy link
Member

@roberth roberth commented Dec 6, 2023 •
edited
Loading

Motivation

Allows the whitelisting of entire schemes. Besides hydra's use of the feature, this allows for instance the exclusion of http: by listing out the other schemes.

2.19:

$ nix run nix/2.19.2 -- eval --expr 'builtins.fetchTree { type = "github"; owner = "NixOS"; repo = "nix"; rev = "50f8f1c8bc019a4c0fd098b9ac674b94cfc6af0d"; }' \
    --restrict-eval --allowed-uris "github:"
error:
...
       error: access to URI 'github:NixOS/nix/50f8f1c8bc019a4c0fd098b9ac674b94cfc6af0d' is forbidden in restricted mode

Now:

$ outputs/out/bin/nix eval --expr 'builtins.fetchTree { type = "github"; owner = "NixOS"; repo = "nix"; rev = "50f8f1c8bc019a4c0fd098b9ac674b94cfc6af0d"; }' \
    --restrict-eval --allowed-uris "github:"
{ lastModified...

Context

Discussed briefly on matrix.

Priorities

Add 👍 to pull requests you find important.

@roberth roberth added language The Nix expression language; parser, interpreter, primops, evaluation, etc fetching Networking with the outside (non-Nix) world, input locking labels Dec 6, 2023
@thufschmitt thufschmitt added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label Dec 8, 2023
@roberth roberth mentioned this pull request Dec 11, 2023
Merged
@roberth roberth force-pushed the allowed-scheme-without-slash branch from 198a725 to 6873ebb Compare December 11, 2023 11:14
@roberth roberth force-pushed the allowed-scheme-without-slash branch from 6873ebb to a05bc9e Compare December 11, 2023 11:18
@nixos-discourse
Copy link

nixos-discourse commented Dec 11, 2023

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-12-08-nix-team-meeting-minutes-110/36721/1

lheckemann
lheckemann reviewed Dec 11, 2023
View reviewed changes
lheckemann
lheckemann approved these changes Dec 11, 2023
View reviewed changes
Copy link
Member

@lheckemann lheckemann left a comment

Choose a reason for hiding this comment

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

❤️ tests

@lheckemann lheckemann mentioned this pull request Dec 11, 2023
Merged
13 tasks
edolstra
edolstra requested changes Dec 12, 2023
View reviewed changes
@roberth
schemeRegex -> schemeNameRegex
2e451a6 
Scheme could be understood to include the typical `:` separator.
@roberth
isValidSchemeName: Use regex
4eaeda6 
As requested by Eelco Dolstra. I think it used to be simpler.
@roberth
Revert "Add nix::isASCII*, locale-independent"
0b87ba5 
This reverts commit 79eb292.

Not used at this time.
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Dec 12, 2023
@roberth roberth force-pushed the allowed-scheme-without-slash branch from 0f6bd12 to 0b87ba5 Compare December 12, 2023 17:52
@Ericson2314
Copy link
Member

Ericson2314 commented Dec 12, 2023

(FYI #9593 seems perhaps tangentially related, or maybe I am confused.)

@roberth
Copy link
Member Author

roberth commented Dec 12, 2023 •
edited
Loading

@Ericson2314 Some things might come to light when refactoring libfetchers to an immutable style.

@roberth roberth requested a review from edolstra December 12, 2023 19:50
@roberth roberth mentioned this pull request Dec 13, 2023
Open
@edolstra edolstra merged commit 1b7968e into NixOS:master Dec 13, 2023
@github-actions
Copy link

github-actions bot commented Dec 13, 2023

Backport failed for 2.19-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.19-maintenance
git worktree add -d .worktree/backport-9547-to-2.19-maintenance origin/2.19-maintenance
cd .worktree/backport-9547-to-2.19-maintenance
git switch --create backport-9547-to-2.19-maintenance
git cherry-pick -x 91ba7b230777e3fb023bda48c269d533702e50e8 6cbba914a70eb5da6447fee5528a63723ed13245 1fa958dda1ef0cb37441ef8d1a84faf6d501ac12 79eb2920bb51c7ec9528a403986e79f04738e2be d3a85b68347071d8d93ec796a38c707483d7b272 a05bc9eb92371af631fc9fb83c3595957fb56943 2e451a663eff96b89360cfd3c0d5eaa60ca46181 4eaeda6604e2f8977728f14415fe92350d047970 0b87ba50c08d83384e11a8e6db1e2f97fba4b61c

@roberth roberth mentioned this pull request Dec 13, 2023
Merged
roberth added a commit that referenced this pull request Dec 13, 2023
@roberth
Merge pull request #9609 from NixOS/backport-9547-to-2.19-maintenance
b38e5a6 
[Backport 2.19-maintenance] `allowed-uris`: match whole schemes without slashes #9547
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Dec 14, 2023

Naive question:
Shouldn't allowed-uris = [ "git+https://gitea.example.de/example/module.git" ] allow git+https://gitea.example.de/example/module.git?ref=refs/heads/master&rev=347b23112b8cdff1a8f2638f6eae072e77cf0139 to be fetched? I've applied the backport PR to hydra and my previous issue with github: got fixed but now I am stuck with that.

@roberth
Copy link
Member Author

roberth commented Feb 2, 2024

That is a reasonable expectation @SuperSandro2000.

Also git+ssh should allow user@authority:path Git URLs.

@nixos-discourse
Copy link

nixos-discourse commented Feb 6, 2024

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-2-20-released/39027/9

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

Reviewers

@Ericson2314 Ericson2314 Ericson2314 left review comments

@edolstra edolstra edolstra approved these changes

@fricklerhandwerk fricklerhandwerk Awaiting requested review from fricklerhandwerk

+1 more reviewer

@lheckemann lheckemann lheckemann approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@edolstra edolstra

Labels

fetching Networking with the outside (non-Nix) world, input locking idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. language The Nix expression language; parser, interpreter, primops, evaluation, etc with-tests Issues related to testing. PRs with tests have some priority

Projects

Nix team
Status: Done

Milestone

fetch-tree stabilisation

Development

Successfully merging this pull request may close these issues.

8 participants

@roberth @nixos-discourse @Ericson2314 @SuperSandro2000 @lheckemann @edolstra @fricklerhandwerk @thufschmitt