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

bake: change evaluation of entitlement paths #2860

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

tonistiigi
Copy link
Member

Currently, to compare the local path used by bake against the paths allowed by entitlements, symlinks were evaluated for path normalization so that the local path used by build was allowed to not exist while the path allowed by entitlement needed to exist. If the path used by the build did not exist, then the deepest existing parent path was used instead. This was concistent with entitlement rules as that parent path would be the actual path access is needed.

This raised an issue with --set if one provides a non-existing path as an argument, as these paths are supposed to be allowed automatically. With the above restrictions set to allowed paths, this meant the build would fail as it can't grant entitlement to the non-existing paths.

This changes the evaluation logic for allowing paths so that they do not need to exist. If such a case appears, then the path is evaluated to the last component that exists, and then the rest of the path is appended as is.

This means that for example, if output = /tmp/out/foo/ is set in HCL and /tmp is the last component that exists then invoking build with --allow fs.write=/tmp/out/foo will not fail with stat error anymore but will fail in entitlements validation as build would also need to write /tmp/out that is not inside the allowed /tmp/out/foo path. The same would apply to --set as well so that if it points to a non-existing path, then an additional --allow rule is needed providing access to writing to the last existing component of that path. This may or may not be unexpected.

@tonistiigi tonistiigi added this to the v0.19.3 milestone Dec 16, 2024
@tonistiigi tonistiigi force-pushed the entitlements-path-validation-fix branch from 2515d64 to bb4161c Compare December 16, 2024 17:22
@crazy-max crazy-max force-pushed the entitlements-path-validation-fix branch from 621d5f6 to bb4161c Compare December 16, 2024 21:05
@tonistiigi tonistiigi force-pushed the entitlements-path-validation-fix branch from bb4161c to 6eb8eeb Compare December 17, 2024 06:21
@tonistiigi tonistiigi marked this pull request as ready for review December 17, 2024 06:21
Currently, to compare the local path used by bake against the paths allowed
by entitlements, symlinks were evaluated for path normalization so that the
local path used by build was allowed to not exist while the path allowed by
entitlement needed to exist. If the path used by the build did not exist,
then the deepest existing parent path was used instead. This was concistent
with entitlement rules as that parent path would be the actual path access
is needed.

This raised an issue with `--set` if one provides a non-existing path as
an argument, as these paths are supposed to be allowed automatically. With
the above restrictions set to allowed paths, this meant the build would fail
as it can't grant entitlement to the non-existing paths.

This changes the evaluation logic for allowing paths so that they do not
need to exist. If such a case appears, then the path is evaluated to the
last component that exists, and then the rest of the path is appended as is.

This means that for example, if `output = /tmp/out/foo/` is set in HCL
and `/tmp` is the last component that exists then invoking build with
`--allow fs.write=/tmp/out/foo` will not fail with stat error anymore
but will fail in entitlements validation as build would also need to
write `/tmp/out` that is not inside the allowed `/tmp/out/foo` path. The
same would apply to `--set` as well so that if it points to
a non-existing path, then an additional `--allow` rule is needed
providing access to writing to the last existing component of that path.
This may or may not be unexpected.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi force-pushed the entitlements-path-validation-fix branch from 6eb8eeb to a53ed0a Compare December 17, 2024 06:29
@crazy-max crazy-max merged commit 21b1be1 into docker:master Dec 17, 2024
122 checks passed
@crazy-max crazy-max modified the milestones: v0.19.3, v0.20.0 Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants