Lock down file permissions #1770
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carolynvs
commented
Sep 21, 2021
| @@ -62,7 +84,7 @@ func TestPorter_Build(t *testing.T) { | |||
|
|
|||
| stamp, err := configadapter.LoadStamp(*bun) | |||
| require.NoError(t, err) | |||
| assert.Equal(t, "d421a6249dfbdba79e26e866da7533d59590565708dfdb32423cf989f588d0ea", stamp.ManifestDigest) | |||
| assert.NotEmpty(t, stamp.ManifestDigest) | |||
There was a problem hiding this comment.
This is the most awful and unhelpful check, so I've fixed it to just make sure a stamp exists, but not the exact hash. This fix is already included the release/v1 branch.
carolynvs
force-pushed
the
file-permissions
branch
4 times, most recently
from
fa39417 to
7be3a54
Compare
|
/azp run porter-integration |
|
Azure Pipelines successfully started running 1 pipeline(s). |
carolynvs
force-pushed
the
file-permissions
branch
4 times, most recently
from
3943b2a to
7bc58f1
Compare
|
Oops sorry I accidentally flipped this out of draft but I'm still fixing stuff! |
Only give permission to the current user, never to the group or other. Only mixins/plugins and scripts should be executable. When porter accesses files, it checks that there aren't any credentials/parameteres/claims with wide-open permissions. This check is skipped on Windows because files by default are not shared, and Go can't accurately report file permissions on Windows. $ porter --debug list --debug-plugins Resolved porter binary from /usr/local/bin/porter to /Users/carolynvs/.porter/porter Resolved storage plugin to storage.porter.filesystem /Users/carolynvs/.porter/porter plugin run storage.porter.filesystem PORTER HOME: /Users/carolynvs/.porter Checking file permissions in /Users/carolynvs/.porter Error: could not list installations: could not read storage schema document: incorrect file permissions on /Users/carolynvs/.porter/config.toml, it should be 600. Correct it manually or by running porter storage fix-permissions. $ porter storage fix-permissions Resetting file permissions in /Users/carolynvs/.porter... $ porter --debug list --debug-plugins Resolved porter binary from /usr/local/bin/porter to /Users/carolynvs/.porter/porter Resolved storage plugin to storage.porter.filesystem /Users/carolynvs/.porter/porter plugin run storage.porter.filesystem Checking file permissions in /Users/carolynvs/.porter NAME CREATED MODIFIED LAST ACTION LAST STATUS bug 2021-09-20 2021-09-20 install succeeded porter-hello 2021-09-20 2021-09-20 install failed whalegap 2021-09-14 2021-09-14 install succeeded mas-whales 2021-09-14 2021-09-14 install failed mybuns 2021-08-10 2021-08-10 install succeeded spike-helm3-mysql 2021-08-02 2021-08-02 install succeeded helm3-mysql 2021-07-29 2021-07-29 install succeeded hello-keyvault 2021-07-27 2021-07-27 install succeeded helm-mysql 2021-06-30 2021-06-30 install succeeded tmp 2021-06-30 2021-06-30 upgrade succeeded credentials-tutorial 2021-06-29 2021-06-29 install succeeded porterops 2021-06-10 2021-06-10 install succeeded kubernetes 2020-08-24 2020-08-24 install succeeded HELLO 2020-08-17 2020-08-17 uninstall succeeded Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs
force-pushed
the
file-permissions
branch
from
7bc58f1 to
a4e0aec
Compare
carolynvs
commented
Sep 23, 2021
| @@ -1,29 +1,36 @@ | |||
| // +build integration | |||
There was a problem hiding this comment.
I made this an integration test because it hits the filesystem and relies on build having been run first.
|
/azp run porter-integration |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this change
Only give permission to the current user, never to the group or other. Only mixins/plugins and scripts should be executable.
Note that on Windows, Go can't accurately report file permissions. Since files by default on Windows are not shared with other users, I am excluding windows from the permission check.
What issue does it fix
Closes #1765
Notes for the reviewer
At this point the cnab-go version that porter v0.38 uses and the one that porter v1 use have diverged quite a bit. I have a branch of cnab-go in porter's fork that lets me make small patches just for our stable branch.
I did not submit the cnab-go patch that we are using in this PR here upstream to cnab-go because that code is going to be deleted very soon, because the crud package is being removed from cnab-go entirely. There's no point it patching it upstream.
getporter/cnab-go@59a143b
Checklist