Skip to content

Commit 1dd235c

Browse files
committed
fix: Safer file path handling for the service
To avoid path traversal the service will only operate within its work directory.
1 parent 46d69a2 commit 1dd235c

18 files changed

+249
-100
lines changed

AGENTS.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ make build # Build to bin/sectool
1616
make build-cross # Cross-compile (linux/darwin, amd64/arm64)
1717
make test # Quick tests (-short flag)
1818
make test-all # Full tests with -race and coverage
19-
make test-cover # Generate HTML coverage report (test.out)
2019
make lint # Run golangci-lint and go vet
2120
```
2221

@@ -256,7 +255,7 @@ All endpoints over Unix socket at `.sectool/service/socket`:
256255
- Test case names should be at most 3 to 5 words and in lower case with underscores
257256
- Assertions rely on `testify` (`require` for setup, `assert` for assertions)
258257
- With assertions don't include messages unless the message provides context outside of the test point, or the two variables being evaluated
259-
- Always verify with `make test` and `make lint` before considering changes complete
258+
- Always verify with `make test-all` and `make lint` before considering changes complete
260259
- Isolated temp directories via `t.TempDir()`
261260
- Context timeouts via `t.Context()`
262261
- Mock MCP server available via `service/mcp.NewTestMCPServer()`

sectool/proxy/export.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"github.com/jentfoo/llm-security-toolbox/sectool/service"
1111
)
1212

13-
func export(timeout time.Duration, flowID, out string) error {
13+
func export(timeout time.Duration, flowID string) error {
1414
ctx, cancel := context.WithTimeout(context.Background(), timeout)
1515
defer cancel()
1616

@@ -26,7 +26,6 @@ func export(timeout time.Duration, flowID, out string) error {
2626

2727
resp, err := client.ProxyExport(ctx, &service.ProxyExportRequest{
2828
FlowID: flowID,
29-
OutDir: out,
3029
})
3130
if err != nil {
3231
return fmt.Errorf("proxy export failed: %w", err)

sectool/proxy/flags.go

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,18 +118,15 @@ proxy export <flow_id>
118118
Note: Prefer 'replay send --flow' with modification flags for simple changes.
119119
Export is useful for complex edits (raw body, binary data, etc).
120120
121-
Creates bundle in .sectool/requests/<id>/:
121+
Creates bundle in .sectool/requests/<bundle_id>/:
122122
request.http HTTP headers with body placeholder
123123
body request body (edit this for modifications)
124124
request.meta.json metadata (method, URL, timestamps)
125125
126-
Options:
127-
--out <path> custom output directory
128-
129126
Examples:
130127
sectool proxy list --host example.com # find flow_id
131-
sectool proxy export f7k2x # outputs: .sectool/requests/f7k2x/
132-
sectool replay send --bundle .sectool/requests/f7k2x
128+
sectool proxy export f7k2x # exports to .sectool/requests/<bundle_id>/
129+
sectool replay send --bundle <bundle_id> # replay the exported bundle
133130
134131
Output: Bundle path and files created
135132
@@ -300,10 +297,8 @@ func parseExport(args []string) error {
300297
fs := pflag.NewFlagSet("proxy export", pflag.ContinueOnError)
301298
fs.SetInterspersed(true)
302299
var timeout time.Duration
303-
var out string
304300

305301
fs.DurationVar(&timeout, "timeout", 30*time.Second, "client-side timeout")
306-
fs.StringVar(&out, "out", "", "output directory (default: .sectool/requests/<auto>)")
307302

308303
fs.Usage = func() {
309304
fmt.Fprint(os.Stderr, `Usage: sectool proxy export <flow_id> [options]
@@ -338,7 +333,7 @@ Options:
338333
return errors.New("flow_id required (get from 'sectool proxy list' with filters)")
339334
}
340335

341-
return export(timeout, fs.Args()[0], out)
336+
return export(timeout, fs.Args()[0])
342337
}
343338

344339
// TODO - planned intercept feature

sectool/replay/flags.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ Workflow:
4545
3. Or export, edit files, then replay:
4646
sectool proxy export <flow_id>
4747
# edit .sectool/requests/<bundle_id>/body
48-
sectool replay send --bundle .sectool/requests/<bundle_id>
48+
sectool replay send --bundle <bundle_id>
4949
5050
---
5151
@@ -55,7 +55,7 @@ replay send [options]
5555
5656
Input sources (exactly one required):
5757
--flow <flow_id> replay from proxy history
58-
--bundle <path> replay from exported bundle directory
58+
--bundle <bundle_id> replay from exported bundle (from proxy export)
5959
--file <path> replay from raw HTTP file (- for stdin)
6060
6161
Request modifications (combine multiple):
@@ -88,7 +88,7 @@ replay send [options]
8888
sectool replay send --flow f7k2x --set-header "Authorization: Bearer tok"
8989
sectool replay send --flow f7k2x --path /api/v2/users --set-query "id=123"
9090
sectool replay send --flow f7k2x --set-json "user.role=admin"
91-
sectool replay send --bundle .sectool/requests/abc123
91+
sectool replay send --bundle abc123
9292
sectool replay send --file request.http --body payload
9393
9494
Output: Markdown with replay_id, status, headers, body preview
@@ -119,7 +119,7 @@ func parseSend(args []string) error {
119119

120120
fs.DurationVar(&timeout, "timeout", 30*time.Second, "client-side timeout")
121121
fs.StringVar(&flow, "flow", "", "flow_id to replay from proxy history")
122-
fs.StringVar(&bundle, "bundle", "", "path to request bundle directory")
122+
fs.StringVar(&bundle, "bundle", "", "bundle_id from proxy export")
123123
fs.StringVar(&file, "file", "", "path to request.http file (- for stdin)")
124124
fs.StringVar(&body, "body", "", "path to body file (use with --file)")
125125
fs.StringVar(&target, "target", "", "override target URL (scheme://host:port)")
@@ -142,7 +142,7 @@ Send a request through the HTTP backend.
142142
143143
Input sources (exactly one required):
144144
--flow <flow_id> Replay from proxy history (get flow_id from 'sectool proxy list')
145-
--bundle <path> Replay from exported bundle (create with 'sectool proxy export')
145+
--bundle <bundle_id> Replay from exported bundle (create with 'sectool proxy export')
146146
--file <path> Replay from raw HTTP file (- for stdin)
147147
148148
File format (--file):

sectool/replay/replay.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,15 @@ func send(timeout time.Duration, flow, bundle, file, body, target string, header
6969
return fmt.Errorf("failed to start service: %w (check %s)", err, client.LogPath())
7070
}
7171

72+
// Extract base name for backward compat with full paths
73+
bundleID := bundle
74+
if bundle != "" {
75+
bundleID = filepath.Base(bundle)
76+
}
77+
7278
req := &service.ReplaySendRequest{
7379
FlowID: flow,
74-
BundlePath: bundle,
80+
BundleID: bundleID,
7581
FilePath: file,
7682
BodyPath: body,
7783
Target: target,

sectool/service/backend_oast_interactsh_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ func TestInteractshBackend_CreateAndClose(t *testing.T) {
1313
if testing.Short() {
1414
t.Skip("skipping integration test in short mode")
1515
}
16+
t.Parallel()
1617

1718
backend := NewInteractshBackend()
1819
t.Cleanup(func() { _ = backend.Close() })
@@ -59,6 +60,7 @@ func TestInteractshBackend_PollSession(t *testing.T) {
5960
if testing.Short() {
6061
t.Skip("skipping integration test in short mode")
6162
}
63+
t.Parallel()
6264

6365
backend := NewInteractshBackend()
6466
t.Cleanup(func() { _ = backend.Close() })
@@ -278,7 +280,7 @@ func TestInteractshBackend_PollSession(t *testing.T) {
278280
done <- pollResult{result, err}
279281
}()
280282

281-
time.Sleep(50 * time.Millisecond)
283+
time.Sleep(20 * time.Millisecond)
282284
sess.mu.Lock()
283285
sess.events = append(sess.events, OastEventInfo{
284286
ID: "new_event",

sectool/service/handler_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import (
1010
)
1111

1212
// testServerWithMCP creates a test server with mock MCP and returns cleanup func.
13-
func testServerWithMCP(t *testing.T) (*Server, *TestMCPServer) {
13+
// Returns server, mock MCP, and the workDir for creating test files within bounds.
14+
func testServerWithMCP(t *testing.T) (*Server, *TestMCPServer, string) {
1415
t.Helper()
1516

1617
mockMCP := NewTestMCPServer(t)
@@ -34,7 +35,7 @@ func testServerWithMCP(t *testing.T) (*Server, *TestMCPServer) {
3435
<-serverErr
3536
})
3637

37-
return srv, mockMCP
38+
return srv, mockMCP, workDir
3839
}
3940

4041
// doRequest is a helper to make HTTP requests to the server.

sectool/service/ids/ids.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,3 +30,20 @@ func Generate(length int) string {
3030

3131
return string(result)
3232
}
33+
34+
// IsValid returns true if the ID contains only valid base62 characters.
35+
// Used to validate user-supplied IDs to prevent path traversal.
36+
func IsValid(id string) bool {
37+
if id == "" {
38+
return false
39+
}
40+
for _, c := range id {
41+
isDigit := c >= '0' && c <= '9'
42+
isUpper := c >= 'A' && c <= 'Z'
43+
isLower := c >= 'a' && c <= 'z'
44+
if !isDigit && !isUpper && !isLower {
45+
return false
46+
}
47+
}
48+
return true
49+
}

sectool/service/ids/ids_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,37 @@ func TestGenerate(t *testing.T) {
5454
})
5555
}
5656

57+
func TestIsValid(t *testing.T) {
58+
t.Parallel()
59+
60+
tests := []struct {
61+
name string
62+
id string
63+
valid bool
64+
}{
65+
{"valid_alphanumeric", "abc123XYZ", true},
66+
{"valid_numbers", "123456", true},
67+
{"valid_lowercase", "abcdef", true},
68+
{"valid_uppercase", "ABCDEF", true},
69+
{"valid_generated", Generate(DefaultLength), true},
70+
{"empty", "", false},
71+
{"with_slash", "abc/def", false},
72+
{"with_backslash", "abc\\def", false},
73+
{"with_dot", "abc.def", false},
74+
{"with_dotdot", "..", false},
75+
{"path_traversal", "../etc/passwd", false},
76+
{"with_space", "abc def", false},
77+
{"with_dash", "abc-def", false},
78+
{"with_underscore", "abc_def", false},
79+
}
80+
81+
for _, tc := range tests {
82+
t.Run(tc.name, func(t *testing.T) {
83+
assert.Equal(t, tc.valid, IsValid(tc.id))
84+
})
85+
}
86+
}
87+
5788
func BenchmarkGenerate(b *testing.B) {
5889
for range b.N {
5990
Generate(DefaultLength)

0 commit comments

Comments
 (0)