Skip to content

Fix running hydraJobs in CI #494

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.

Sign up for GitHub

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

Merged
edolstra merged 7 commits into from
Jun 10, 2026
Merged

Fix running hydraJobs in CI #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
604e107
Fix running hydraJobs in CI
edolstra Jun 10, 2026
258d02d
Run VM tests for one variant only
edolstra Jun 10, 2026
933ad82
Revert "fetchTarball: Remove custom substitution code"
xokdvium Jun 10, 2026
ced7241
tests/nixos/s3-binary-cache-store.nix: Fix bad merge
edolstra Jun 10, 2026
b638394
Ensure that tests in not empty
edolstra Jun 10, 2026
8397d69
Remove upgrade-nix tests
edolstra Jun 10, 2026
fa8e406
Use default kernel
edolstra Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,20 @@ jobs:
- uses: DeterminateSystems/determinate-nix-action@main
- uses: DeterminateSystems/flakehub-cache-action@main
- run: |
tests=$(nix flake show --json \
| jq -r '
.inventory.hydraJobs.output.children.tests.children
| with_entries(select(.value | has("derivation")))
| keys[]
| "${{ inputs.flake }}#hydraJobs.tests." + .')

@coderabbitai coderabbitai Bot Jun 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Harden against code injection via template expansion and unquoted variable usage.

Line 138 embeds ${{ inputs.flake }} into the jq output, and line 146 uses $tests unquoted. If inputs.flake contains shell metacharacters (e.g., ; or |), they will be interpreted by the shell at line 146, enabling command injection.

While the threat is mitigated because this is a workflow_call (only trusted repo workflows can supply inputs), defense-in-depth is warranted.

🔒 Proposed fix using a bash array

Replace lines 133–146 with:

-      - run: |
-          tests=$(nix flake show --json \
-            | jq -r '
-              .inventory.hydraJobs.output.children.tests.children
-              | with_entries(select(.value | has("derivation")))
-              | keys[]
-              | "${{ inputs.flake }}`#hydraJobs.tests`." + .')
-
-          if [ -z "$tests" ]; then
-            echo "error: no tests found in hydraJobs.tests"
-            exit 1
-          fi
-
-          cmd() {
-            nix build -L --keep-going --timeout 600 $tests
-          }
+      - run: |
+          mapfile -t tests < <(
+            nix flake show --json \
+              | jq -r '
+                .inventory.hydraJobs.output.children.tests.children
+                | with_entries(select(.value | has("derivation")))
+                | keys[]
+                | "${{ inputs.flake }}`#hydraJobs.tests`." + .'
+          )
+
+          if [ ${`#tests`[@]} -eq 0 ]; then
+            echo "error: no tests found in hydraJobs.tests"
+            exit 1
+          fi
+
+          cmd() {
+            nix build -L --keep-going --timeout 600 "${tests[@]}"
+          }

This uses mapfile to safely populate an array and "${tests[@]}" to expand elements without shell interpretation of metacharacters.

Also applies to: 146-146

🧰 Tools
🪛 zizmor (1.25.2)

[error] 138-138: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/build.yml at line 138, The workflow embeds `${{
inputs.flake }}` into a jq expression and later expands `$tests` unquoted,
allowing shell metacharacters in inputs to be interpreted; change the pipeline
to have jq emit a JSON array (rather than raw/unquoted strings) and then read
that JSON safely into a bash array (e.g., via mapfile/readarray + jq -r '.[]' or
using jq -c to produce an array and jq -r '.[]' into mapfile) and always expand
the results as `"${tests[@]}"`; in short, stop inserting `${{ inputs.flake }}`
raw into shell text, emit/consume JSON-safe values from jq, populate `tests`
with mapfile/readarray, and use quoted expansion (`"${tests[@]}"`) wherever
`$tests` is used.

Source: Linters/SAST tools


if [ -z "$tests" ]; then
echo "error: no tests found in hydraJobs.tests"
exit 1
fi

cmd() {
nix build -L --keep-going --timeout 600 \
$(nix flake show --json \
| jq -r '
.hydraJobs.tests
| with_entries(select(.value.type == "derivation"))
| keys[]
| "${{ inputs.flake }}#hydraJobs.tests." + .')
nix build -L --keep-going --timeout 600 $tests
}

if ! cmd; then
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ jobs:
runner_for_virt: UbuntuLatest32Cores128G
runner_small: ubuntu-latest
run_tests: true
run_vm_tests: true
run_vm_tests: false
run_regression_tests: true
upload_artifacts: false

Expand Down
29 changes: 25 additions & 4 deletions src/libexpr/primops/fetchTree.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,17 +427,38 @@ static void fetch(
.atPos(pos)
.debugThrow();

// early exit if pinned and already in the store
if (expectedHash && expectedHash->algo == HashAlgorithm::SHA256) {
auto expectedPath = state.store->makeFixedOutputPath(
name,
FixedOutputInfo{
.method = unpack ? FileIngestionMethod::NixArchive : FileIngestionMethod::Flat,
.hash = *expectedHash,
.references = {}});

// Try to get the path from the local store or substituters
try {
state.store->ensurePath(expectedPath);
debug("using substituted/cached path '%s' for '%s'", state.store->printStorePath(expectedPath), *url);
state.allowAndSetStorePathString(expectedPath, v);
return;
} catch (Error & e) {
debug(
"substitution of '%s' failed, will try to download: %s",
state.store->printStorePath(expectedPath),
e.what());
// Fall through to download
}
}

if (unpack) {
auto attrs = fetchers::Attrs{
{"type", "tarball"},
{"url", *url},
{"name", name},
};
if (expectedHash) {
if (expectedHash)
attrs.emplace("narHash", expectedHash->to_string(HashFormat::SRI, true));
// Mark as final to allow the tree to be substituted.
attrs.emplace("__final", Explicit<bool>{true});
}
auto input = fetchers::Input::fromAttrs(state.fetchSettings, std::move(attrs));
auto cachedInput =
state.inputCache->getAccessor(state.fetchSettings, *state.store, input, fetchers::UseRegistries::No);
Expand Down
17 changes: 0 additions & 17 deletions tests/nixos/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,21 +205,4 @@ in
fetchersSubstitute = runNixOSTest ./fetchers-substitute.nix;

chrootStore = runNixOSTest ./chroot-store.nix;

upgrade-nix = runNixOSTest {
imports = [ ./upgrade-nix.nix ];
upgrade-nix.oldNix = nixComponents.nix-cli;
};

upgrade-nix_fromStable = runNixOSTest {
imports = [ ./upgrade-nix.nix ];
name = lib.mkForce "upgrade-nix-from-stable";
upgrade-nix.oldNix = pkgs.nixVersions.stable;
};

upgrade-nix_fromLatest = runNixOSTest {
imports = [ ./upgrade-nix.nix ];
name = lib.mkForce "upgrade-nix-from-latest";
upgrade-nix.oldNix = pkgs.nixVersions.latest;
};
}
94 changes: 0 additions & 94 deletions tests/nixos/s3-binary-cache-store.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -953,100 +953,6 @@ in
)
verify_packages_in_store(client, PKGS['A'])

@setup_s3(
populate_bucket=[PKGS['A']],
profiles={
"valid": {"access_key": ACCESS_KEY, "secret_key": SECRET_KEY},
"invalid": {"access_key": "INVALIDKEY", "secret_key": "INVALIDSECRET"},
}
)
def test_profile_credentials(bucket):
"""Test that profile-based credentials work without environment variables"""
print("\n=== Testing Profile-Based Credentials ===")

store_url = make_s3_url(bucket, profile="valid")

# Verify store info works with profile credentials (no env vars)
client.succeed(f"HOME=/root nix store info --store '{store_url}' >&2")

# Verify we can copy from the store using profile
verify_packages_in_store(client, PKGS['A'], should_exist=False)
client.succeed(f"HOME=/root nix copy --no-check-sigs --from '{store_url}' {PKGS['A']}")
verify_packages_in_store(client, PKGS['A'])

# Clean up the package we just copied so we can test invalid profile
client.succeed(f"nix store delete --ignore-liveness {PKGS['A']}")
verify_packages_in_store(client, PKGS['A'], should_exist=False)

# Verify invalid profile fails when trying to copy
invalid_url = make_s3_url(bucket, profile="invalid")
client.fail(f"HOME=/root nix copy --no-check-sigs --from '{invalid_url}' {PKGS['A']} 2>&1")

@setup_s3(
populate_bucket=[PKGS['A']],
profiles={
"wrong": {"access_key": "WRONGKEY", "secret_key": "WRONGSECRET"},
}
)
def test_env_vars_precedence(bucket):
"""Test that environment variables take precedence over profile credentials"""
print("\n=== Testing Environment Variables Precedence ===")

# Use profile with wrong credentials, but provide correct creds via env vars
store_url = make_s3_url(bucket, profile="wrong")

# Ensure package is not in client store
verify_packages_in_store(client, PKGS['A'], should_exist=False)

# This should succeed because env vars (correct) override profile (wrong)
output = client.succeed(
f"HOME=/root {ENV_WITH_CREDS} nix copy --no-check-sigs --debug --from '{store_url}' {PKGS['A']} 2>&1"
)

# Verify the credential chain shows Environment provider was added
if "Added AWS Environment Credential Provider" not in output:
print("Debug output:")
print(output)
raise Exception("Expected Environment provider to be added to chain")

# Clean up the package so we can test again without env vars
client.succeed(f"nix store delete --ignore-liveness {PKGS['A']}")
verify_packages_in_store(client, PKGS['A'], should_exist=False)

# Without env vars, same URL should fail (proving profile creds are actually wrong)
client.fail(f"HOME=/root nix copy --no-check-sigs --from '{store_url}' {PKGS['A']} 2>&1")

@setup_s3(
populate_bucket=[PKGS['A']],
profiles={
"testprofile": {"access_key": ACCESS_KEY, "secret_key": SECRET_KEY},
}
)
def test_credential_provider_chain(bucket):
"""Test that debug logging shows which providers are added to the chain"""
print("\n=== Testing Credential Provider Chain Logging ===")

store_url = make_s3_url(bucket, profile="testprofile")

output = client.succeed(
f"HOME=/root nix store info --debug --store '{store_url}' 2>&1"
)

# For a named profile, we expect to see these providers in the chain
expected_providers = ["Environment", "Profile", "IMDS"]
for provider in expected_providers:
msg = f"Added AWS {provider} Credential Provider to chain for profile 'testprofile'"
if msg not in output:
print("Debug output:")
print(output)
raise Exception(f"Expected to find: {msg}")

# SSO should be skipped (no SSO config for this profile)
if "Skipped AWS SSO Credential Provider for profile 'testprofile'" not in output:
print("Debug output:")
print(output)
raise Exception("Expected SSO provider to be skipped")

# ============================================================================
# Main Test Execution
# ============================================================================
Expand Down
104 changes: 0 additions & 104 deletions tests/nixos/upgrade-nix.nix
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion tests/nixos/user-sandboxing/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ in
create-hello-world
pkgs.socat
];
boot.kernelPackages = pkgs.linuxPackages_latest;
users.users.alice = {
isNormalUser = true;
};
Expand Down
Loading