fix symlink resolution for noble #200
Conversation
cf-foundation-community-automation
bot
moved this to Inbox
in Foundational Infrastructure Working Group
| . "github.com/onsi/ginkgo/v2" | ||
| . "github.com/onsi/gomega" | ||
|
|
||
| . "bpm/runc/adapter" |
There was a problem hiding this comment.
nit: generally try to avoid dot-imports (ginkgo / gomega being an exception)
There was a problem hiding this comment.
nit: this can probably be src/bpm/runc/adapter/mount_test.go since there are unlikely to be more mount_{xyz}_test.go files.
aramprice
left a comment
aramprice
left a comment
There was a problem hiding this comment.
Thank you for the changes, this look good code wise. I would appreciate some other folks looking at it as well, especially to think over any potential security changes from this.
Before this change we allowed an inside-container-mount of an outside-the-container-symlink to resolve at container runtime, from inside the container to the outside-container-symlink-target.
With this change bpm will pre-resolve, not at container runtime, an outside-the-container-symlink to its outside-the-container-symlink-target (not mediated by the container subsystem), and then an inside-the-container-mount will be created which points directly to the outside-the-container-symlink-target
I think this change introduces a difference in the time at which a symlink is resolved (container-creation vs. ad hoc during container runtime)... and I'm curious if this is consequential to the security posture of bpm. I don't think this is the case for the issue mentioned in #199...
I'm wondering if this could have implications in arbitrary cases?
| var ( | ||
| tempDir string | ||
| realDir string | ||
| symlinkPath string |
There was a problem hiding this comment.
nit: to better reveal intention perhaps this var could be symlinkToRealDir?
| err = os.Mkdir(realDir, 0755) | ||
| Expect(err).NotTo(HaveOccurred()) | ||
|
|
||
| symlinkPath = filepath.Join(tempDir, "symlink") |
There was a problem hiding this comment.
nit: similar to the var naming, this value could be "symlink-to-real-directory"
2 participants
Fixes #199