Skip to content

ci: skip the binary cache setup when credentials are unavailable - #236

Merged
rlahfa-dinum merged 1 commit into
cloud-gouv:mainfrom
RISK-alt:fix/testsuite-cache-on-fork-prs
Aug 14, 2026
Merged

ci: skip the binary cache setup when credentials are unavailable#236
rlahfa-dinum merged 1 commit into
cloud-gouv:mainfrom
RISK-alt:fix/testsuite-cache-on-fork-prs

Conversation

@RISK-alt

@RISK-alt RISK-alt commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Problem

The [Sécurix] Test suite workflow fails on every pull request opened from a fork, regardless of the change it carries:

post-build-hook: error: AWS error fetching 'nix-cache-info': Unable to parse ExceptionName: InvalidRequest Message: Invalid Request.
error:
       … while running the post-build-hook /tmp/setup-nix-cache-action-…/post-build-hook for derivation /nix/store/…-test-script.drv
       error: program '/tmp/setup-nix-cache-action-…/post-build-hook' failed with exit code 123

GitHub does not expose repository secrets to workflows triggered by pull_request from a fork, so NIX_SIGNING_PRIVATE_KEY, AWS_ACCESS_KEY_ID and AWS_SECRET_KEY are all empty strings. The S3 store then rejects every request, and the post-build hook takes nix-build down with it.

Two things make this hard to read from the outside:

  • the failure comes from the upload hook, not from the build, so the job is red even though the derivations built fine;
  • the job is green on internal branches, which makes it look like the contributed change is at fault.

Affected pull requests I could find: #228, #230, #231, #234, #235. Internal branches such as #233 are unaffected.

Change

Guard the cache setup on the presence of the credentials. The secrets context is not available in an if: condition, so the check goes through a job-level env entry, which does have access to it.

  • internal branches and pushes to main: unchanged, the cache is still read from and written to;
  • forks: the cache step is skipped and the build runs against cache.nixos.org alone, so the job reports the actual result of the test suite.

Note

docs/manual/src/user/cache.md already recommends separating an untrusted CI cache from a trusted CD cache for projects accepting external contributions. That would be the more complete answer here, but it needs bucket and secret changes that only maintainers can make. This patch only stops fork pull requests from reporting a failure they did not cause, and does not stand in the way of that approach.

Verification

Since pull_request runs the workflow definition carried by the pull request, this one exercises the change directly: the fork path of the condition is what its own Test suite run goes through. The runs are held in action_required because the pull request touches .github/workflows, so approving them is the verification. A green Test suite here is the result the fix is meant to produce; the credentials-present path stays covered by main after merge.

Checked before opening:

  • the S3 endpoint returns HTTP 400 for an unauthenticated read of nix-cache-info, confirming the bucket is unusable as a substituter without credentials, and that no read-only fallback is available;
  • the secrets context is unavailable in jobs.<job_id>.steps.if but available in jobs.<job_id>.env, and the env context is available in steps.if, per the contexts reference;
  • secret_keys is what drives the upload in zombiezen/setup-nix-cache-action, and the credentials are needed for downloads too, so skipping the whole step is the correct granularity;
  • the file still parses as valid YAML with the expected condition on the expected step.

GitHub does not expose repository secrets to workflows triggered by a pull
request opened from a fork. The three inputs given to the cache action are
then empty strings, and the S3 store rejects every request:

    post-build-hook: error: AWS error fetching 'nix-cache-info':
    Unable to parse ExceptionName: InvalidRequest Message: Invalid Request.
    error: program '.../post-build-hook' failed with exit code 123

The failure comes from the upload hook rather than from the build itself, so
the test suite reports a failure even though the derivations built fine. Every
pull request coming from a fork has been affected.

Guard the cache setup on the presence of the credentials. Internal branches and
pushes to main keep reading from and writing to the cache, while forks build
against cache.nixos.org alone and report the actual result of the test suite.
@rlahfa-dinum

Copy link
Copy Markdown
Contributor

Hi there, are you using AI for your changes?

@RISK-alt

Copy link
Copy Markdown
Contributor Author

Hi there, are you using AI for your changes?

Hello, I don't use AI for coding, especially when it comes to “sensitive data.” However, I do use it to review my changes in case there are any errors, and especially for the content of my pull requests. This is to follow the guidelines in contributing.md and avoid making mistakes.

@rlahfa-dinum

Copy link
Copy Markdown
Contributor

Hi there, are you using AI for your changes?

Hello, I don't use AI for coding, especially when it comes to “sensitive data.” However, I do use it to review my changes in case there are any errors, and especially for the content of my pull requests. This is to follow the guidelines in contributing.md and avoid making mistakes.

Can you point me where do you see credentials errors in the CI?

@RISK-alt

RISK-alt commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

The OVH endpoint answers an unsigned request with 400 InvalidRequest rather than 403 AccessDenied, so it surfaces as a parse failure on an unexpected error body. The evidence is in the difference between a run that has the secrets and one that does not.

1. The action never receives the inputs.

Push on main, run 31386493613:

##[group]Run zombiezen/setup-nix-cache-action@cacc7abf...
with:
  substituters: s3://oss-securix?endpoint=...
  secret_keys: ***
  aws_access_key_id: ***
  aws_secret_access_key: ***
  use_nixcached: false

PR #235 from a fork, run 31264063350:

##[group]Run zombiezen/setup-nix-cache-action@cacc7abf...
with:
  substituters: s3://oss-securix?endpoint=...
  use_nixcached: false

The three secret-backed inputs are gone. The runner omits with: entries that resolve to an empty string.

2. No credentials file is written.

Post-job cleanup on main:

Removing /etc/systemd/system/nix-daemon.service.d/aws-credentials.conf...

That line does not appear anywhere in the fork run, whose cleanup only removes the /tmp/setup-nix-cache-action-… directory.

3. The substituter is unusable as a result.

main reads from it, e.g. copying path '/nix/store/…-vm-test-run-minimal' from 's3://oss-securix', 4 paths total. The fork run hits warning: AWS error fetching 'nix-cache-info' on first access, pulls 0 paths from s3://oss-securix, and falls back to cache.nixos.org entirely. The post-build hook then aborts the build on the first derivation it tries to upload, about two minutes in and well before any VM test runs, which is why the job is red without a single test having executed.

As a cross-check, an unauthenticated curl https://s3.gra.io.cloud.ovh.net/oss-securix/nix-cache-info also returns 400, which matches what the runner sees and is why this patch skips the step entirely rather than keeping the bucket as a read-only substituter for forks.

To be explicit about what this does not claim: I am not saying the suite goes green on forks, only that the job would report its actual result instead of failing in the upload hook. Since the hook kills the build before any test executes, I have no data on the real outcome.

@RISK-alt

Copy link
Copy Markdown
Contributor Author

And I'm more than willing to discuss my point of view.

@rlahfa-dinum

Copy link
Copy Markdown
Contributor

Thank you for elaborating. You are right on (1) & (2). On (3), it's definitely weird, we had intended this cache to be public, so I will double check on the infrastructure side what's going on. Your curl example doesn't return access denied, it returns invalid request ("Not S3 Request"), if you write in vhost-style, it will return access denied.

It would be helpful for our future engagements if you can disclose your use of AI and how do you use it and where. Some stuff registers as AI prose sometimes and AI prose makes it hard to understand whether it's you who made this analysis or a tool that assisted you.

@RISK-alt

Copy link
Copy Markdown
Contributor Author

Thank you for elaborating. You are right on (1) & (2). On (3), it's definitely weird, we had intended this cache to be public, so I will double check on the infrastructure side what's going on. Your curl example doesn't return access denied, it returns invalid request ("Not S3 Request"), if you write in vhost-style, it will return access denied.

It would be helpful for our future engagements if you can disclose your use of AI and how do you use it and where. Some stuff registers as AI prose sometimes and AI prose makes it hard to understand whether it's you who made this analysis or a tool that assisted you.

Yes, no problem at all. I'd be happy to explain this to you in any way you prefer—on GitHub, via email, or during a class or meeting.

@rlahfa-dinum
rlahfa-dinum merged commit 3ffe6e0 into cloud-gouv:main Aug 14, 2026
3 of 4 checks 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.

2 participants