Repository navigation
Build context fix, bump to containerization 0.52.1. - #2415
Merged
Merged
Conversation
`BuildFSSync.read()` and `info()` resolved a requested path, checked that the result was inside the build context, and then opened the original path. A symlink replaced between the check and the open was followed to its new target, which could be outside the context. This change opens each requested file by walking its path one component at a time from a directory descriptor pinned on the context root, using `FileDescriptorOps` from `containerization`. Symlinks are followed only when the target is relative and stays beneath the context root. The metadata and the content returned to the builder both come from the same open descriptor. Behavior change: a symlink with an absolute target is now rejected by `read()` and `info()`, even when the target is inside the context. Relative symlinks that stay inside the context still work. `walk()`-based transfers are unchanged. Dependency: bumps `containerization` from 0.49.0 to 0.50.0 for the `FileDescriptorOps` API used here. Tests, in `BuildFSSyncTests`: - Regular files and relative in-context symlinks are served. - A directory read returns no data. - A symlink repeatedly swapped between an in-context and an out-of-context target is never served from the out-of-context target. - `read()` and `info()` reject an absolute symlink whose target is inside the context.
katiewasnothere
approved these changes
Oct 19, 2026
Code Coverage
|
6 tasks done
benoitf
pushed a commit
to socktainer/socktainer
that referenced
this pull request
Oct 20, 2026
) * deps: bump apple/container to 1.6.0 and containerization to 0.52.1 apple/container 1.6.0 pins containerization 0.52.1. The one source change it needs is in ExtraHostsFile: NetworkClient.lookup(hostname:) is now lookup(name:), and it returns a NetworkMatch wrapping the Attachment, since a name can now also match an attachment's DNS alias (apple/container#2390). That lookup's XPC message changed with it: a 1.6.0 network plugin reads only the new `name` key and a 1.5.0 one reads only `hostname`. A socktainer built against one version cannot look up attachments on the other, so socktainer and Apple Container have to move together. The release also brings, with no change here: - A fix to BuildFSSync.read()/info(), which checked a context path and then opened it again, so a symlink swapped in between could escape the build context (apple/container#2415). Symlinks with absolute targets are now rejected on that fallback path. - container-builder-shim 0.13.2 as the default builder image. - Sparse OCI images load on import (apple/containerization#961). Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Wes Morgan <git@wesmorgan.me> * refactor(builder): start BuildKit with apple/container's Builder.start apple/container 1.6.0 moved the builder bootstrap that `container build` uses out of the CLI and into the ContainerBuild library as Builder.start (apple/container#2370). ClientBuilderService had its own copy, which has drifted from upstream before: #260 was the copy missing upstream's capAdd = ["ALL"]. Hand the non-running cases to Builder.start instead. It creates the "buildkit" container, restarts a stopped one, and recreates one whose image, CPUs, memory or DNS no longer match the system configuration. That last part is new for socktainer, which never replaced an existing builder before. A builder created before an upgrade now moves to the new default image the next time it has to be started. A builder that is already running is still used as it is, so a build in progress is never interrupted. Behavior differences from the old copy: - CPUs and memory come from the [build] section of the system configuration (default 2 CPUs and 2048MB, the values that were hard-coded here), so socktainer and `container build` agree on the shared builder instead of each recreating it to its own settings. - The nameserver is no longer set explicitly. The runtime fills in the network's gateway when none is given, the same address socktainer computed. - A builder in an unknown state is bootstrapped again instead of being deleted first. If that fails, Builder.start deletes it and the next request creates a new one. Builder.start fetches the image over HTTPS only, so socktainer still fetches it first with the scheme it picks for the reference. A builder image on a local insecure registry keeps working, and Builder.start finds it already in the store. The builderContainerConfiguration tests go with the code they covered. Those settings are now upstream's. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Wes Morgan <git@wesmorgan.me> * fix(builder): don't fail a builder restart on the image pre-fetch startBuilder fetched the builder image, with the scheme its reference calls for, before handing over to Builder.start, and failed if that fetch did. Builder.start restarts a stopped builder that still matches without touching its image, which may since have been deleted (Apple Container only protects images of running containers), so an unreachable registry would fail a restart that needs no image. Make the pre-fetch best effort and log its failure: Builder.start still fails, with its own fetch, when it does need the image. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Wes Morgan <git@wesmorgan.me> * ci: retrigger integration test The previous two runs' integration jobs failed on infrastructure: a dropped kernel download, then no macOS runner acquired. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Wes Morgan <git@wesmorgan.me> --------- Signed-off-by: Wes Morgan <git@wesmorgan.me> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
BuildFSSync.read()andinfo()resolved a requested path, checked that the result was inside the build context, and then opened the original path. A symlink replaced between the check and the open was followed to its new target, which could be outside the context.-This change opens each requested file by walking its path one component at a time from a directory descriptor pinned on the context root, using
FileDescriptorOpsfromcontainerization. Symlinks are followed only when the target is relative and stays beneath the context root. The metadata and the content returned to the builder both come from the same open descriptor.read()andinfo(), even when the target is inside the context. Relative symlinks that stay inside the context still work.walk()-based transfers are unchanged.containerization0.50.0 for itsFileDescriptorOpsAPI enhancements. This change updates the library from 0.52.0 to 0.52.1.Type of Change
Testing