Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADD/COPY do not strip paths outside of the build context #21827

Open
1 task done
cowwoc opened this issue Jan 15, 2025 · 7 comments
Open
1 task done

ADD/COPY do not strip paths outside of the build context #21827

cowwoc opened this issue Jan 15, 2025 · 7 comments
Labels
status/triage Needs triage

Comments

@cowwoc
Copy link

cowwoc commented Jan 15, 2025

Is this a docs issue?

  • My issue is about the documentation content or website

Type of issue

Information is incorrect

Description

@dvdksn It looks like the paragraph you changed at https://github.com/moby/buildkit/pull/5664/files#r1916876722 is wrong.

I just tested against Docker version 27.4.0, build bde2b89 and both ADD and COPY commands return this error if you try referencing paths outside of the build context:

ADD failed: forbidden path outside the build context: ../src/main/resources/scripts/helm-install.sh ()

Frankly, I prefer this behavior. It's a lot less error-prone. Can you please update the documentation accordingly?

Location

https://docs.docker.com/reference/dockerfile/#adding-files-from-the-build-context

Suggestion

No response

@cowwoc cowwoc added the status/triage Needs triage label Jan 15, 2025
@cowwoc
Copy link
Author

cowwoc commented Jan 15, 2025

I stand corrected. The docker engine spits out the aforementioned error, but the docker CLI happily accepts the input files. I am investigating the created image to see what it ended up doing...

@cowwoc
Copy link
Author

cowwoc commented Jan 15, 2025

Okay. It does what you mentioned... If ADD/COPY reference files outside the build context, the CLI will silently drop the leading ../ and process the corrected path.

On the flip side, it is a bit disturbing that the behavior of the CLI and Engine do not line up...

@thaJeztah Do you plan to leave the behavior as-is, or change it?

@thaJeztah
Copy link
Member

Was this with buildkit enabled, or the classic builder? https://github.com/moby/moby/blob/b1c00b18bd8ac2603e37321f825034ce4d191522/builder/remotecontext/archive.go#L125

The classic builder had a check for this, but ISTR I had a lengthy discussion with the buildkit maintainers who considered that ../../../../ outside the root was better to be match the behavior on Linux itself, and ignore when trying to get outside the root.

See

@cowwoc
Copy link
Author

cowwoc commented Jan 15, 2025

@thaJeztah I'm using Docker Desktop on Windows with this config:

{
  "builder": {
    "gc": {
      "defaultKeepStorage": "20GB",
      "enabled": true
    }
  },
  "experimental": false
}

so I believe this means I am using the classic builder. I invoked docker build . helm/Dockefile.

@dvdksn
Copy link
Collaborator

dvdksn commented Jan 15, 2025

Are you building Windows containers? Docker in Windows mode still uses the legacy (non-BuildKit) builder. Otherwise, with Linux containers, you would be using BuildKit by default (since Docker Engine v23.0).

@dvdksn
Copy link
Collaborator

dvdksn commented Jan 15, 2025

Good news is that BuildKit is making progress in Windows mode too, so hopefully soon it will be used on all platforms. You can use it already today (experimental) if you run BuildKit as a separate process.

https://docs.docker.com/build/buildkit/#buildkit-on-windows

@cowwoc
Copy link
Author

cowwoc commented Jan 16, 2025

@dvdksn I am building a Linux container, so I guess I am using BuildKit...

Regardless, shouldn't the server and CLI provide the same behavior? I would expect both the client (CLI) and server to know what build mode I am using and provide the same behavior when faced with excessive ../ in the path...

Is that possible to implement?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/triage Needs triage
Projects
None yet
Development

No branches or pull requests

3 participants