Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stdenv/setup.sh: make sure $sourceRoot has +x before cd-ing #117357

Merged
merged 1 commit into from Mar 24, 2022

Conversation

sternenseemann
Copy link
Member

@sternenseemann sternenseemann commented Mar 23, 2021

This change is prompted by the following, admittedly cursed tarball:

> curl https://registry.npmjs.org/char-regex/-/char-regex-1.0.2.tgz 2>/dev/null \
  | tar -ztv
drw-rw-rw- 0/0               0 2020-02-18 10:50 package
-rw-rw-rw- 0/0             297 2020-02-18 10:50 package/index.d.ts
-rw-rw-rw- 0/0            1920 2020-02-18 10:50 package/index.js
-rw-rw-rw- 0/0            1092 2020-01-31 11:31 package/LICENSE
-rw-rw-rw- 0/0             937 2020-02-18 10:51 package/package.json
-rw-rw-rw- 0/0             713 2020-02-18 10:50 package/README.md

The minimal reproducer for the issue is the following derivation trying
to work around the uid 0 issue with dontMakeSourcesWritable = true:

{ stdenv, fetchurl }:

stdenv.mkDerivation {
  name = "test";

  src = fetchurl {
    sha1 = "d744358226217f981ed58f479b1d6bcc29545dcf";
    url = "https://registry.npmjs.org/char-regex/-/char-regex-1.0.2.tgz";
  };

  dontMakeSourcesWritable = true;

  installPhase = ''
    cp -R . $out
  '';
}

This currently fails in the following way:

these derivations will be built:
  /nix/store/pc3jbydl0xcc8nrndf5xkf7hdhpgpb41-test.drv
building '/nix/store/pc3jbydl0xcc8nrndf5xkf7hdhpgpb41-test.drv'...
unpacking sources
unpacking source archive /nix/store/v9p98kqplf4kflmy91p0687xlvr6klb1-char-regex-1.0.2.tgz
source root is package
find: 'package/index.d.ts': Permission denied
find: 'package/index.js': Permission denied
find: 'package/LICENSE': Permission denied
find: 'package/package.json': Permission denied
find: 'package/README.md': Permission denied
/nix/store/6c47azxacncswc1pllzj28zfzqw40d7c-stdenv-linux/setup: line 1311: cd: package: Permission denied
builder for '/nix/store/pc3jbydl0xcc8nrndf5xkf7hdhpgpb41-test.drv' failed with exit code 1
error: build of '/nix/store/pc3jbydl0xcc8nrndf5xkf7hdhpgpb41-test.drv' failed

As you can see, the issue is that $sourceRoot isn't executable,
prohibiting the call to cd. This can be fixed by running
chmod +x "${sourceRoot}" before cd regardless of
dontMakeSourcesWritable in unpackPhase since if chmod fails, cd
would fail as well and we are out of options.

Verified that the workaround works locally.

Another thing to investigate is investigating if we should use
--no-same-owner for tar and if it helps in this case as well.
See also Profpatsch/yarn2nix#56.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

This change is prompted by the following, admittedly cursed tarball:

```
> curl https://registry.npmjs.org/char-regex/-/char-regex-1.0.2.tgz 2>/dev/null \
  | tar -ztv
drw-rw-rw- 0/0               0 2020-02-18 10:50 package
-rw-rw-rw- 0/0             297 2020-02-18 10:50 package/index.d.ts
-rw-rw-rw- 0/0            1920 2020-02-18 10:50 package/index.js
-rw-rw-rw- 0/0            1092 2020-01-31 11:31 package/LICENSE
-rw-rw-rw- 0/0             937 2020-02-18 10:51 package/package.json
-rw-rw-rw- 0/0             713 2020-02-18 10:50 package/README.md
```

The minimal reproducer for the issue is the following derivation trying
to work around the uid 0 issue with `dontMakeSourcesWritable = true`:

```nix
{ stdenv, fetchurl }:

stdenv.mkDerivation {
  name = "test";

  src = fetchurl {
    sha1 = "d744358226217f981ed58f479b1d6bcc29545dcf";
    url = "https://registry.npmjs.org/char-regex/-/char-regex-1.0.2.tgz";
  };

  dontMakeSourcesWritable = true;

  installPhase = ''
    cp -R . $out
  '';
}
```

This currently fails in the following way:

```
these derivations will be built:
  /nix/store/pc3jbydl0xcc8nrndf5xkf7hdhpgpb41-test.drv
building '/nix/store/pc3jbydl0xcc8nrndf5xkf7hdhpgpb41-test.drv'...
unpacking sources
unpacking source archive /nix/store/v9p98kqplf4kflmy91p0687xlvr6klb1-char-regex-1.0.2.tgz
source root is package
find: 'package/index.d.ts': Permission denied
find: 'package/index.js': Permission denied
find: 'package/LICENSE': Permission denied
find: 'package/package.json': Permission denied
find: 'package/README.md': Permission denied
/nix/store/6c47azxacncswc1pllzj28zfzqw40d7c-stdenv-linux/setup: line 1311: cd: package: Permission denied
builder for '/nix/store/pc3jbydl0xcc8nrndf5xkf7hdhpgpb41-test.drv' failed with exit code 1
error: build of '/nix/store/pc3jbydl0xcc8nrndf5xkf7hdhpgpb41-test.drv' failed
```

As you can see, the issue is that `$sourceRoot` isn't executable,
prohibiting the call to `cd`. This can be fixed by running
`chmod +x "${sourceRoot}"` before `cd` regardless of
`dontMakeSourcesWritable` in `unpackPhase` since if `chmod` fails, `cd`
would fail as well and we are out of options.

Verified that the workaround works locally.

Another thing to investigate is investigating if we should use
`--no-same-owner` for `tar` and if it helps in this case as well.
See also <Profpatsch/yarn2nix#56>.
@stale
Copy link

stale bot commented Sep 21, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md and removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Sep 21, 2021
@Ericson2314
Copy link
Member

Ericson2314 commented Mar 24, 2022

This is fine with me. I am not worried about unpackPhase becoming more complex, as I would like Nixpkgs to eventually make unpackPhase opt-in as more packages support out-of-tree builds.

@sternenseemann sternenseemann merged commit b5cad4d into NixOS:staging Mar 24, 2022
@sternenseemann sternenseemann deleted the setup-exec-before-cd branch March 24, 2022 10:13
@Artturin Artturin mentioned this pull request Jul 30, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants