Skip to content

fix(unikontainers): secure container state paths - #973

Open
tagadearpit wants to merge 1 commit into
urunc-dev:mainfrom
tagadearpit:fix/secure-container-state-paths
Open

fix(unikontainers): secure container state paths#973
tagadearpit wants to merge 1 commit into
urunc-dev:mainfrom
tagadearpit:fix/secure-container-state-paths

Conversation

@tagadearpit

@tagadearpit tagadearpit commented Aug 14, 2026

Copy link
Copy Markdown

Summary

Fixes a symlink-based escape in container-state path construction tracked by #791.

New, Get, and the delete-command fallback previously used filepath.Join(rootDir, containerID). Although container IDs reject traversal characters, a valid ID can still name a symlink that points outside rootDir. State reads and cleanup operations could then follow that symlink outside the configured container-state directory.

This change uses the existing github.com/cyphar/filepath-securejoin dependency for all three path-construction sites and returns a resolution error before reading or removing state. It also adds TestGetRejectsContainerDirSymlink, which creates a symlinked container directory and verifies that Get cannot access the outside state file. The authenticated contributor is included in .github/contributors.yaml as required by the project contribution guide.

Related issues

How was this tested?

The following checks pass:

GOTOOLCHAIN=go1.26.4 CGO_ENABLED=1 go test ./pkg/unikontainers -run '^TestGetRejectsContainerDirSymlink$' -count=1 -v
go test ./pkg/unikontainers ./cmd/urunc -count=1
go test ./cmd/... ./internal/... ./pkg/... -count=1
go vet ./cmd/... ./internal/... ./pkg/...
go test ./... -run '^$' -count=1

A full go test ./... run reaches the external e2e suites but cannot complete in this environment because ctr, nerdctl, docker, and crictl are unavailable. The repository-wide compile-only test pass succeeds.

LLM usage

This pull request was prepared with assistance from OpenAI GPT-5. The contributor reviewed the generated changes, reproduced the reported behavior, and ran the relevant tests and static checks.

Checklist

  • I have read the contribution guide.
  • The linter passes locally (make lint); the repository’s container-based linter is unavailable in this environment.
  • The e2e tests of at least one tool pass locally (make test_ctr, make test_nerdctl, make test_docker, make test_crictl); the required external tools are unavailable in this environment.
  • If LLMs were used: I have read the LLM policy.

Replace filepath.Join with SecureJoin for container state directories so symlinks cannot redirect state access outside the configured root. Add regression coverage for Get and harden the delete fallback path.

Fixes: urunc-dev#791
Signed-off-by: tagadearpit <[email protected]>
@netlify

netlify Bot commented Aug 14, 2026

Copy link
Copy Markdown

Deploy Preview for urunc canceled.

Name Link
🔨 Latest commit 55231d7
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/6a7f1ac8940dd9000855b767

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.

use securejoin.SecureJoin instead of filepath.Join for containerID path construction

1 participant