fix: address CodeRabbit review issues
- Fix critical sandbox escape in local.Medium.path() - Absolute paths now constrained to sandbox root when root != "/" - Only allow absolute path passthrough when root is "/" - Fix weak test assertion in TestMust_Ugly_Panics - Use assert.Contains instead of weak OR condition - Remove unused issues.json file - Add TestPath_RootFilesystem test for absolute path handling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
4b0d89ca7c
commit
cf4cbd44d8
4 changed files with 20 additions and 5 deletions
File diff suppressed because one or more lines are too long
|
|
@ -25,14 +25,20 @@ func New(root string) (*Medium, error) {
|
|||
|
||||
// path sanitizes and returns the full path.
|
||||
// Replaces .. with . to prevent traversal, then joins with root.
|
||||
// Absolute paths are only allowed when root is "/", otherwise they are
|
||||
// treated as relative to the sandbox root.
|
||||
func (m *Medium) path(p string) string {
|
||||
if p == "" {
|
||||
return m.root
|
||||
}
|
||||
clean := strings.ReplaceAll(p, "..", ".")
|
||||
if filepath.IsAbs(clean) {
|
||||
// Only allow absolute paths to pass through if root is "/"
|
||||
// Otherwise, strip leading "/" to constrain to sandbox
|
||||
if filepath.IsAbs(clean) && m.root == "/" {
|
||||
return filepath.Clean(clean)
|
||||
}
|
||||
// Strip leading "/" for sandboxed mediums
|
||||
clean = strings.TrimPrefix(clean, "/")
|
||||
return filepath.Join(m.root, clean)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -29,8 +29,19 @@ func TestPath(t *testing.T) {
|
|||
assert.Equal(t, "/home/user/file.txt", m.path("../file.txt"))
|
||||
assert.Equal(t, "/home/user/dir/file.txt", m.path("dir/../file.txt"))
|
||||
|
||||
// Absolute paths pass through
|
||||
// Absolute paths are constrained to sandbox (no escape)
|
||||
assert.Equal(t, "/home/user/etc/passwd", m.path("/etc/passwd"))
|
||||
}
|
||||
|
||||
func TestPath_RootFilesystem(t *testing.T) {
|
||||
m := &Medium{root: "/"}
|
||||
|
||||
// When root is "/", absolute paths pass through
|
||||
assert.Equal(t, "/etc/passwd", m.path("/etc/passwd"))
|
||||
assert.Equal(t, "/home/user/file.txt", m.path("/home/user/file.txt"))
|
||||
|
||||
// Relative paths still work
|
||||
assert.Equal(t, "/file.txt", m.path("file.txt"))
|
||||
}
|
||||
|
||||
func TestReadWrite(t *testing.T) {
|
||||
|
|
|
|||
|
|
@ -3,7 +3,6 @@ package log
|
|||
import (
|
||||
"bytes"
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
|
@ -270,5 +269,5 @@ func TestMust_Ugly_Panics(t *testing.T) {
|
|||
|
||||
// Verify error was logged before panic
|
||||
output := buf.String()
|
||||
assert.True(t, strings.Contains(output, "[ERR]") || len(output) > 0)
|
||||
assert.Contains(t, output, "[ERR]", "Should log error before panic")
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue