cmd: use os.Root for data directories in compact, receive, and sidecar#8726
Closed
Retr0-XD wants to merge 1 commit into
Closed
cmd: use os.Root for data directories in compact, receive, and sidecar#8726Retr0-XD wants to merge 1 commit into
Retr0-XD wants to merge 1 commit into
Conversation
Author
|
Hi @bwplotka @GiedriusS — would appreciate a review when you get a chance! |
Retr0-XD
force-pushed
the
feat/use-os-root-for-data-dirs-8103
branch
from
March 24, 2026 18:34
8f05536 to
f7e12d3
Compare
Use os.Root (introduced in Go 1.24) to open restricted file-system
roots for the data directories in the Compactor, Receiver, and Sidecar
components. File operations performed through os.Root are bounded to
the named directory and cannot escape via path traversal (e.g. '../').
Compactor (compact.go):
- Ensure conf.dataDir exists before opening the root.
- Replace os.MkdirAll for the 'compact' and 'downsample' subdirs with
dataRoot.Mkdir so those operations are also root-restricted.
Receiver (receive.go):
- Ensure conf.dataDir exists with os.MkdirAll before opening the
root; this is a no-op when the directory already exists.
Sidecar (sidecar.go):
- Open a restricted root for conf.tsdb.path immediately after the
existing IsWALDirAccessible validation confirms the path is present.
Fixes thanos-io#8103
Signed-off-by: Retr0-XD <[email protected]>
Retr0-XD
force-pushed
the
feat/use-os-root-for-data-dirs-8103
branch
from
March 24, 2026 18:34
f7e12d3 to
db95c01
Compare
Author
|
@GiedriusS @bwplotka can you review this PR :) |
GiedriusS
reviewed
Mar 25, 2026
GiedriusS
left a comment
GiedriusS
left a comment
Member
There was a problem hiding this comment.
Good first issues are for humans, not llms. I don't think you understood the changes that the bot made for you.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 this PR does / Why we need it
Closes #8103.
Uses
os.Root(introduced in Go 1.24) to open restricted file-system roots for the configured data directories in the Compactor, Receiver, and Sidecar components. File operations performed through an*os.Rootare bounded to the named directory and cannot escape via path traversal (e.g.../). This prevents any future code from accidentally introducing a path-traversal vulnerability by accepting malicious or malformed directory input.Changes
cmd/thanos/compact.goconf.dataDirexists (os.MkdirAll) before opening the root (no-op when the directory already exists).*os.Rootforconf.dataDiranddeferits close.os.MkdirAllcalls for thecompactanddownsamplesubdirs withdataRoot.Mkdir, so those operations are also root-restricted.cmd/thanos/receive.goconf.dataDirexists withos.MkdirAllat startup (no-op when it already exists).*os.Rootforconf.dataDiranddeferits close.cmd/thanos/sidecar.go"os"to imports.*os.Rootforconf.tsdb.pathimmediately after the existingIsWALDirAccessiblecheck (which confirms the directory is accessible), anddeferits close.Which issue(s) this PR fixes
Fixes #8103
AI disclosure: GitHub Copilot was used during implementation. I fully understand all changes made in this PR.