Conversation
… compression (cherry picked from commit 2ccb8a9)
…-2.33-maintenance [Backport 2.33-maintenance] upload-release: disable containerd image store to preserve gzip layer compression
Generalize writeToStderr into writeFullLogging, with similar behavior but taking an arbitrary fd, and reimplement writeToStderr with it as a convenient wrapper. (cherry picked from commit 473d54e)
Logging should not check for interrupts. Use the new writeFullLogging function to write JSON output to get similar behavior to SimpleLogger. This avoids the logger itself throwing Interrupted exceptions in handleExceptions. (cherry picked from commit 1210100)
…tenance [Backport 2.33-maintenance] Don't throw Interrupted from JSONLogger::write
(cherry picked from commit 2334977)
…tenance [Backport 2.33-maintenance] Fix the nix-community/patsh/0.2.1 flake regression test (again)
RemoteFSAccessor::fetch() crashes when called with the root path. $ nix build nixpkgs#hello --store ssh-ng://host error: path '/nix/store/' is not in the Nix store Add path.isRoot() guard like LocalStoreAccessor::maybeLstat Fixes: eb643d0 ("`Store::getFSAccessor`: Do not include the store dir") (cherry picked from commit b523564)
Resolves NixOS#15420 Signed-off-by: Lisanna Dettwyler <lisanna.dettwyler@gmail.com> (cherry picked from commit 342faaa)
…tenance [Backport 2.33-maintenance] libstore: handle root path in RemoteFSAccessor::maybeLstat
…tenance [Backport 2.33-maintenance] Fix compatibility with lowdown 3
…hain Nix 2.33 replaced aws-sdk-cpp with aws-crt-cpp for S3 credentials. The custom credential chain (Environment → SSO → Profile → IMDS) omitted the STS WebIdentity provider that was part of the old DefaultAWSCredentialsProviderChain, breaking EKS IRSA, GitHub Actions OIDC, and any other sts:AssumeRoleWithWebIdentity flow. The observed failure mode on EKS pods using IRSA was a misleading 'Valid credentials could not be sourced by the IMDS provider' error — IMDS is merely the last provider tried after WebIdentity was skipped. aws-c-auth (already linked) exposes aws_credentials_provider_new_sts_web_identity, which reads AWS_WEB_IDENTITY_TOKEN_FILE, AWS_ROLE_ARN, AWS_ROLE_SESSION_NAME, and AWS_REGION from the environment (falling back to the profile config) and returns nullptr at construction time when they aren't set. This is wired into the chain after Profile and before IMDS, matching the pre-2.33 ordering. (cherry picked from commit 9ae4b50)
Same regression class as the preceding commit. The pre-2.33 aws-sdk-cpp credential chain included the ECS container metadata provider, which reads AWS_CONTAINER_CREDENTIALS_RELATIVE_URI (or _FULL_URI) and is used by ECS tasks and EKS Pod Identity. aws_credentials_provider_new_ecs_from_environment reads the relevant env vars and returns nullptr at construction time when none are set, so this slots into the chain with zero runtime cost when unused. Wired after STS WebIdentity and before IMDS, matching the pre-2.33 ordering. The full credential chain is now: Environment → SSO → Profile → STS WebIdentity → ECS → IMDS (cherry picked from commit eec6d92)
(cherry picked from commit ddd84dc)
…tenance [Backport 2.33-maintenance] fix(libstore/aws-creds): add STS WebIdentity and ECR metadata
This fixes a a destructor ordering issue where during shutdown, ~ProgressBar() calls getWindowSize() after the windowSize object (and thus its mutex) has been destroyed. This happens when a draw() call happens to be active during destruction. Fixes #361, NixOS#14300, NixOS#14361. (cherry picked from commit cf44b7c)
…tenance [Backport 2.33-maintenance] Don't destroy windowSize mutex
While debugging NixOS#15564 I noticed that our release tarball don't seem to be reproducible. This doesn't explain why hydra calculated the different hash of the tarball, which should only be built once (???). Regardless, I noticed that checking the build reproducibility against hydra built tarballs fails with e.g. nix build .\#hydraJobs.binaryTarball.x86_64-linux -L --keep-failed --rebuild Looking with diffoscope, it was the entry ordering issue. Let's nip this in the bud. (cherry picked from commit 4c41c5f)
…tenance [Backport 2.33-maintenance] packaging: Make binaryTarball more reproducible
This error no longer seems to occur on Linux 6.19+ anymore. Skip in that case to fix build. (cherry picked from commit 2e6a03e)
…tenance [Backport 2.33-maintenance] tests/functional/stale-file-handle: Skip if the error doesn't happen
(cherry picked from commit 0e3412a)
…e output in a temporary directory in the store Puts the temporary FOD output copies in a temporary directory inside the store instead of the (for Linux sandboxed builds) chroot. This prevents file overwrite due to symlink following that std::filesystem::copy_file does. Also applies the same output copying approach for impure derivations that don't have network sandboxing and thus are subject to FD smuggling. Fixes GHSA-g3g9-5vj6-r3gj. (cherry picked from commit a760af8)
…ew enough kernels This partially fixes the issue with cooperating processes being able to communicate via abstract sockets. The fix is partial, because processes outside the landlock domain of the sandboxed process can still connect to a socket created by the FOD. There's no equivalent way of restricting inbound connections. This closes the gap when there's no cooperating process on the host (i.e. 2 separate FODs). >= 6.12 kernel is widespread enough (NixOS 25.11 ships it by default) that we have no reason not to apply this hardening, even though it's incomplete. ca-fd-leak test exercises this exact code path and now the smuggling process fails with (on new enough kernels that have landlock support enabled): vm-test-run-ca-fd-leak> machine # sandbox setup: applied landlock sandboxing vm-test-run-ca-fd-leak> machine # building '/nix/store/s7brgi6pdr5f3n8yqlgmdlz8blb89njc-smuggled.drv'... vm-test-run-ca-fd-leak> machine # building derivation '/nix/store/s7brgi6pdr5f3n8yqlgmdlz8blb89njc-smuggled.drv': woken up vm-test-run-ca-fd-leak> machine # connect: Operation not permitted vm-test-run-ca-fd-leak> machine # sendmsg: Socket not connected (cherry picked from commit 44017ca)
Tagging release 2.33.4
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdates version to 2.33.4; documents S3 credential-chain changes; makes tarball creation deterministic; adds lowdown v3 detection and fixes markdown flags; extends AWS credential provider chain to include STS WebIdentity and ECS with conditional IMDS fallback; refactors logging writes; minor terminal and remote FS accessor tweaks; build change for sentry-native debug info. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AwsCreds as AwsCredsLogic
participant Env as Environment
participant SSO
participant Profile
participant WebID as STS_WebIdentity
participant ECS
participant IMDS
Client->>AwsCreds: request credentials
AwsCreds->>Env: check Environment
alt found
Env-->>AwsCreds: creds
else
AwsCreds->>SSO: check SSO
alt found
SSO-->>AwsCreds: creds
else
AwsCreds->>Profile: check Profile
alt found
Profile-->>AwsCreds: creds
else
AwsCreds->>WebID: attempt STS WebIdentity (if TLS ok)
alt found
WebID-->>AwsCreds: creds
else
AwsCreds->>ECS: attempt ECS container creds (if TLS ok & env set)
alt ECS added
ECS-->>AwsCreds: creds
else
AwsCreds->>IMDS: fallback to IMDS (only if ECS not added)
IMDS-->>AwsCreds: creds or error
end
end
end
end
end
AwsCreds-->>Client: credentials or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
cole-h
previously approved these changes
Apr 23, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
doc/manual/rl-next/s3-credential-chain-web-identity.md (1)
17-19: Consider adding TLS-context failure as an additional root cause.This section currently points to omitted providers only. In code, TLS context creation failure also disables SSO/STS WebIdentity/ECS, which can lead to the same fallback symptom. A short note here would improve troubleshooting.
✍️ Suggested doc tweak
-The typical symptom was a misleading IMDS error +The typical symptom was a misleading IMDS error (`Valid credentials could not be sourced by the IMDS provider`), because IMDS is the last provider tried after the correct one was skipped. +This can also happen if TLS context initialization fails, which makes +SSO, STS WebIdentity, and ECS providers unavailable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/manual/rl-next/s3-credential-chain-web-identity.md` around lines 17 - 19, Add a short note to the s3-credential-chain-web-identity.md section explaining that TLS context creation failure can disable SSO/STS WebIdentity/ECS credential providers and cause the same fallback symptom ("Valid credentials could not be sourced by the IMDS provider"); update the paragraph that discusses omitted providers to mention "TLS context creation failure" (and optionally "TLS/SSL handshake or certificate load errors") as an additional root cause and include guidance to check TLS initialization logs alongside SSO/STS/WebIdentity/ECS checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@doc/manual/rl-next/s3-credential-chain-web-identity.md`:
- Around line 17-19: Add a short note to the s3-credential-chain-web-identity.md
section explaining that TLS context creation failure can disable SSO/STS
WebIdentity/ECS credential providers and cause the same fallback symptom ("Valid
credentials could not be sourced by the IMDS provider"); update the paragraph
that discusses omitted providers to mention "TLS context creation failure" (and
optionally "TLS/SSL handshake or certificate load errors") as an additional root
cause and include guidance to check TLS initialization logs alongside
SSO/STS/WebIdentity/ECS checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7114e7a4-b79f-468a-8ec2-ab4ed30110ed
📒 Files selected for processing (9)
.versiondoc/manual/rl-next/s3-credential-chain-web-identity.mdpackaging/binary-tarball.nixsrc/libcmd/markdown.ccsrc/libcmd/meson.buildsrc/libstore/aws-creds.ccsrc/libstore/remote-fs-accessor.ccsrc/libutil/logging.ccsrc/libutil/terminal.cc
cole-h
approved these changes
Apr 23, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Context
Summary by CodeRabbit
Bug Fixes
Documentation
Chores / Packaging
Refactor