Skip to content

HCL: add a file() function#2642

Merged
dgageot merged 1 commit intodocker:mainfrom
rumpl:hcl-file
May 6, 2026
Merged

HCL: add a file() function#2642
dgageot merged 1 commit intodocker:mainfrom
rumpl:hcl-file

Conversation

@rumpl
Copy link
Copy Markdown
Member

@rumpl rumpl commented May 5, 2026

Returns the contents of a file

@rumpl rumpl requested a review from a team as a code owner May 5, 2026 21:18
Copy link
Copy Markdown

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

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

Assessment: 🔴 CRITICAL

The new file() built-in function introduces path traversal and unrestricted file access issues. An HCL config can use ../ sequences or absolute paths to read any file the process has permission to access — including credentials, keys, and system files. Three findings: 1 CONFIRMED HIGH, 2 CONFIRMED/LIKELY MEDIUM.

Comment thread pkg/config/hcl/hcl.go Outdated
Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
path := args[0].AsString()
if !filepath.IsAbs(path) && baseDir != "" {
path = filepath.Join(baseDir, path)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[HIGH] Path traversal: file() allows reading arbitrary files via ../ sequences

filepath.Join(baseDir, path) cleanly normalises .. components — for example filepath.Join("/home/user/.config", "../../etc/passwd") resolves to /etc/passwd. There is no check that the resulting path remains within baseDir, so an HCL config can silently read any file the process can access:

instruction = file("../../etc/passwd")

Fix: After joining, verify the resolved path starts with baseDir:

resolved := filepath.Join(baseDir, path)
rel, err := filepath.Rel(baseDir, resolved)
if err != nil || strings.HasPrefix(rel, "..") {
    return cty.NilVal, fmt.Errorf("file path %q escapes config directory", path)
}
path = resolved

Comment thread pkg/config/hcl/hcl.go Outdated
Type: function.StaticReturnType(cty.String),
Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) {
path := args[0].AsString()
if !filepath.IsAbs(path) && baseDir != "" {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[MEDIUM] Absolute paths in file() bypass baseDir anchoring entirely

When the path argument is absolute (e.g. file("/etc/shadow")), the !filepath.IsAbs(path) guard is false and the path is used as-is — baseDir provides zero containment. Combined with the ../ traversal issue above, there is no effective restriction on which files can be read.

If absolute paths are intentional, this should be documented explicitly. If not, add a guard:

if filepath.IsAbs(path) {
    return cty.NilVal, fmt.Errorf("file() does not accept absolute paths")
}

Comment thread pkg/config/hcl/hcl.go
return ctyToGo(val), nil
}

func baseDir(filename string) string {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[MEDIUM] file() silently resolves paths relative to CWD when filename is empty or has no directory component

Two cases:

  1. ToMap(src, "")baseDir("") returns "" → the baseDir != "" guard fails → os.ReadFile resolves relative paths against the process working directory.
  2. ToMap(src, "agent.hcl")filepath.Dir("agent.hcl") returns "."filepath.Join(".", path) still resolves against CWD.

In both cases, file("secret.txt") reads from wherever the binary was invoked, not from a stable, expected location. Consider requiring filename to be an absolute path, or documenting the CWD-relative behaviour explicitly.

Returns the contents of a file

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
@dgageot dgageot merged commit cd03da9 into docker:main May 6, 2026
5 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