Skip to content

Set /home/cnb file mode to 0750 - #286

Merged
sophiewigmore merged 1 commit into
paketo-buildpacks:mainfrom
candrews:home-cnb-mode
Aug 2, 2023
Merged

Set /home/cnb file mode to 0750#286
sophiewigmore merged 1 commit into
paketo-buildpacks:mainfrom
candrews:home-cnb-mode

Conversation

@candrews

@candrews candrews commented Jul 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Set /home/cnb file mode to 750

Use Cases

750 complies with security recommendations such as those made by CIS.

See: https://www.unifiedcompliance.com/products/search-controls/control/01588/

Security scanners are flagging build pack created images due to these too permission file permissions.

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@candrews
candrews requested a review from a team as a code owner July 28, 2023 17:52
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 28, 2023 •
edited
Loading

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: candrews / name: Craig Andrews (ff9fe83)

@candrews candrews mentioned this pull request Jul 28, 2023
Closed
sophiewigmore
sophiewigmore reviewed Aug 1, 2023
View reviewed changes
Comment thread internal/ihop/user_layer_creator.go Outdated
Typeflag: tar.TypeDir,
Name: "home/cnb",
Mode: int64(os.ModePerm),
Mode: int64(os.FileMode(750)),

@sophiewigmore sophiewigmore Aug 1, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey @candrews thanks for the PR! Are you able to address the linting error that's showing up on this line?

@candrews candrews Aug 1, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've pushed an updated to this PR fixing the linting error, thank you for pointing it out.

@candrews
Set /home/cnb file mode to 0750
ff9fe83 
The permission for this directory were 0777 which is too permissive.

0750 complies with security recommendations such as those made by CIS.

Signed-off-by: Craig Andrews <candrews@integralblue.com>
@sophiewigmore sophiewigmore added semver:patch A change requiring a patch version bump and removed semver:patch A change requiring a patch version bump labels Aug 2, 2023
@sophiewigmore

sophiewigmore commented Aug 2, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

@paketo-buildpacks/tooling-maintainers @paketo-buildpacks/stacks-maintainers I'm looking for an opinion on if this change warrants a minor bump or patch -and if we need to be cognizant of the changes to the stack release when this change gets rolled out (cutting minor stack releases)?

@robdimsdale

robdimsdale commented Aug 2, 2023

Copy link
Copy Markdown
Member

This is a tricky one. I'd argue it's a bug fix (and hence semver:patch) because the behavior isn't desired. On the other hand it's a breaking change if you were relying on that behavior. That would lead to semver:major. On the other-other hand, however, I couldn't find any reference in the Buildpacks spec to the permissions of the /home/cnb directory.

So I'm going to argue for semver:patch with the justification that relying on this behavior was an undefined (and hence unsupported) use-case.

@robdimsdale robdimsdale added the semver:patch A change requiring a patch version bump label Aug 2, 2023
@robdimsdale

robdimsdale commented Aug 2, 2023

Copy link
Copy Markdown
Member

@sophiewigmore I labeled it as semver:patch. If you agree - feel free to merge. If you disagree feel free to continue the discussion 😄

@candrews

candrews commented Aug 2, 2023

Copy link
Copy Markdown
Contributor Author

I'm also trying to track down where I can submit a similar change to make the files under /workspace have consistently non-world-write permissions. I raised a discussion at buildpacks/community#229 - perhaps the folks participating in this PR can point me in the right direction?

@robdimsdale

robdimsdale commented Aug 2, 2023

Copy link
Copy Markdown
Member

I pinged a couple of folks in the CNB team on that discussion. I don't have more context unfortunately :/

@sophiewigmore
sophiewigmore merged commit a255494 into paketo-buildpacks:main Aug 2, 2023
@candrews candrews changed the title Set /home/cnb file mode to 750 Set /home/cnb file mode to 0750 Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@sophiewigmore sophiewigmore sophiewigmore approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

semver:patch A change requiring a patch version bump

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@candrews @sophiewigmore @robdimsdale