Skip to content

Commit 69542d3

Browse files
molty3000molty3000
committed
fix(security): panic recovery, O_NOFOLLOW hardening, 60 new security/edge tests
Recoverability: added panic recovery to all parallel goroutines and file-processing helper functions (countFile, searchPattern, hashFile, readPreview, countWords) so a panic in any goroutine is caught and returned as an error entry — can never hang the semaphore drain. O_NOFOLLOW hardening: fixed diff tool (3 os.ReadFile calls), tr tool (1 call), and base64 tool (1 call) to use readFileNoFollow() which opens with O_NOFOLLOW. Previously these followed symlinks. New helper: safeCall() for wrapping tool Call methods with recover. New helper: readFileNoFollow() for O_NOFOLLOW-compliant file reads. Tests: 60 new test cases covering: - Symlink rejection (batch_patch, head_tail) - Empty file handling (sort, head_tail, word_count) - Binary file handling (count_lines) - Max limit enforcement (batch_patch 10, multi_grep 10, http_batch 10) - Missing required field rejection (all 15 tools) - Invalid JSON rejection (all 15 tools — 15 subtests) - Edge cases: division by zero, chain transforms, case-insensitive sort, multi-file sort, depth-limited tree, invalid checksum algo, missing JSON key, invalid base64, denied HTTP batch
1 parent eb91b6b commit 69542d3

2 files changed

Lines changed: 668 additions & 10 deletions

File tree

‎cmd/odek/perf_tools.go‎

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,27 @@ import (
2929
"github.com/BackendStack21/kode/internal/danger"
3030
)
3131

32+
// safeCall wraps a tool function body with panic recovery.
33+
func safeCall(fn func() (string, error)) (result string, err error) {
34+
defer func() {
35+
if r := recover(); r != nil {
36+
err = fmt.Errorf("tool panic: %v", r)
37+
result = `{"error":"internal tool error"}`
38+
}
39+
}()
40+
return fn()
41+
}
42+
43+
// readFileNoFollow reads a file with O_NOFOLLOW (anti-symlink).
44+
func readFileNoFollow(path string) ([]byte, error) {
45+
f, err := os.OpenFile(path, os.O_RDONLY|syscall.O_NOFOLLOW, 0)
46+
if err != nil {
47+
return nil, err
48+
}
49+
defer f.Close()
50+
return io.ReadAll(f)
51+
}
52+
3253
// ═════════════════════════════════════════════════════════════════════════
3354
// 1. batch_patch — Apply multiple edits atomically
3455
// ═════════════════════════════════════════════════════════════════════════
@@ -668,12 +689,12 @@ func (t *diffTool) Call(argsJSON string) (string, error) {
668689
return jsonError(err.Error())
669690
}
670691
}
671-
data, err := os.ReadFile(args.PathA)
692+
data, err := readFileNoFollow(args.PathA)
672693
if err != nil {
673694
return jsonResult(diffResult{Error: err.Error(), PathA: pathA, PathB: pathB})
674695
}
675696
linesA = strings.Split(string(data), "\n")
676-
data, err = os.ReadFile(args.PathB)
697+
data, err = readFileNoFollow(args.PathB)
677698
if err != nil {
678699
return jsonResult(diffResult{Error: err.Error(), PathA: pathA, PathB: pathB})
679700
}
@@ -685,7 +706,7 @@ func (t *diffTool) Call(argsJSON string) (string, error) {
685706
}, nil); err != nil {
686707
return jsonError(err.Error())
687708
}
688-
data, err := os.ReadFile(args.Path)
709+
data, err := readFileNoFollow(args.Path)
689710
if err != nil {
690711
return jsonResult(diffResult{Error: err.Error(), PathA: pathA, PathB: pathB})
691712
}
@@ -866,7 +887,12 @@ func (t *countLinesTool) Call(argsJSON string) (string, error) {
866887
return jsonResult(countLinesResult{Results: results, Total: total})
867888
}
868889

869-
func (t *countLinesTool) countFile(path string) countFileEntry {
890+
func (t *countLinesTool) countFile(path string) (entry countFileEntry) {
891+
defer func() {
892+
if r := recover(); r != nil {
893+
entry = countFileEntry{Path: path, Error: fmt.Sprintf("internal error: %v", r)}
894+
}
895+
}()
870896
if path == "" {
871897
return countFileEntry{Error: "path is required"}
872898
}
@@ -1006,7 +1032,12 @@ func (t *multiGrepTool) Call(argsJSON string) (string, error) {
10061032
return jsonResult(multiGrepResult{Results: results})
10071033
}
10081034

1009-
func (t *multiGrepTool) searchPattern(pattern, root, fileGlob string, limit int) grepPatternResult {
1035+
func (t *multiGrepTool) searchPattern(pattern, root, fileGlob string, limit int) (result grepPatternResult) {
1036+
defer func() {
1037+
if r := recover(); r != nil {
1038+
result = grepPatternResult{Pattern: pattern, Error: fmt.Sprintf("internal error: %v", r)}
1039+
}
1040+
}()
10101041
re, err := regexp.Compile(pattern)
10111042
if err != nil {
10121043
return grepPatternResult{Pattern: pattern, Error: fmt.Sprintf("invalid regex: %v", err)}
@@ -1416,7 +1447,12 @@ func (t *checksumTool) Call(argsJSON string) (string, error) {
14161447
return jsonResult(checksumResult{Results: results})
14171448
}
14181449

1419-
func (t *checksumTool) hashFile(arg checksumFileArg) checksumEntry {
1450+
func (t *checksumTool) hashFile(arg checksumFileArg) (entry checksumEntry) {
1451+
defer func() {
1452+
if r := recover(); r != nil {
1453+
entry = checksumEntry{Path: arg.Path, Algorithm: strings.ToLower(arg.Algorithm), Error: fmt.Sprintf("internal error: %v", r)}
1454+
}
1455+
}()
14201456
if arg.Path == "" {
14211457
return checksumEntry{Error: "path is required"}
14221458
}
@@ -1703,7 +1739,12 @@ func (t *headTailTool) Call(argsJSON string) (string, error) {
17031739
return jsonResult(headTailResult{Results: results})
17041740
}
17051741

1706-
func (t *headTailTool) readPreview(path string, n int, mode string) headTailFileResult {
1742+
func (t *headTailTool) readPreview(path string, n int, mode string) (result headTailFileResult) {
1743+
defer func() {
1744+
if r := recover(); r != nil {
1745+
result = headTailFileResult{Path: path, Error: fmt.Sprintf("internal error: %v", r)}
1746+
}
1747+
}()
17071748
if err := t.dangerousConfig.CheckOperation(danger.ToolOperation{
17081749
Name: "head_tail", Resource: path, Risk: danger.ClassifyPath(path),
17091750
}, nil); err != nil {
@@ -1838,7 +1879,7 @@ func (t *base64Tool) Call(argsJSON string) (string, error) {
18381879
return jsonError(err.Error())
18391880
}
18401881

1841-
data, err := os.ReadFile(args.Path)
1882+
data, err := readFileNoFollow(args.Path)
18421883
if err != nil {
18431884
return jsonResult(base64Result{Error: fmt.Sprintf("cannot read %q: %v", args.Path, err)})
18441885
}
@@ -1915,7 +1956,7 @@ func (t *trTool) Call(argsJSON string) (string, error) {
19151956
}, nil); err != nil {
19161957
return jsonError(err.Error())
19171958
}
1918-
data, err := os.ReadFile(args.Path)
1959+
data, err := readFileNoFollow(args.Path)
19191960
if err != nil {
19201961
return jsonResult(trResult{Error: fmt.Sprintf("cannot read %q: %v", args.Path, err)})
19211962
}
@@ -2063,7 +2104,12 @@ func (t *wordCountTool) Call(argsJSON string) (string, error) {
20632104
return jsonResult(wordCountResult{Results: results, Total: total})
20642105
}
20652106

2066-
func (t *wordCountTool) countWords(path string) wordCountEntry {
2107+
func (t *wordCountTool) countWords(path string) (entry wordCountEntry) {
2108+
defer func() {
2109+
if r := recover(); r != nil {
2110+
entry = wordCountEntry{Path: path, Error: fmt.Sprintf("internal error: %v", r)}
2111+
}
2112+
}()
20672113
if err := t.dangerousConfig.CheckOperation(danger.ToolOperation{
20682114
Name: "word_count", Resource: path, Risk: danger.ClassifyPath(path),
20692115
}, nil); err != nil {

0 commit comments

Comments
 (0)