Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions .github/workflows/cgo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1034,10 +1034,11 @@ jobs:
- name: Lint error messages
run: make lint-errors

# Enforce the production errstringmatch analyzer without blocking on unrelated
# legacy custom analyzer findings in tests or other analyzer families.
# Enforce the production errstringmatch and panicinlibrarycode analyzers without
# blocking on unrelated legacy custom analyzer findings in tests or other analyzer
# families.
- name: Run custom linters
run: make golint-custom LINTER_FLAGS="-errstringmatch -test=false"
run: make golint-custom LINTER_FLAGS="-errstringmatch -panicinlibrarycode -test=false"

actions-build:
runs-on: ubuntu-latest
Expand Down
136 changes: 131 additions & 5 deletions pkg/linters/panic-in-library-code/panic-in-library-code.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ package panicinlibrarycode

import (
"go/ast"
"go/constant"
"go/token"
"go/types"
"strings"

Expand Down Expand Up @@ -36,32 +38,156 @@ func run(pass *analysis.Pass) (any, error) {
(*ast.CallExpr)(nil),
}

insp.Preorder(nodeFilter, func(n ast.Node) {
insp.WithStack(nodeFilter, func(n ast.Node, push bool, stack []ast.Node) bool {
if !push {
return true
}

call := n.(*ast.CallExpr)
// Skip test files
if strings.HasSuffix(pkgPath, ".test") || filecheck.IsTestFile(pass.Fset.Position(call.Pos()).Filename) {
return
return true
}

// Check if this is a call to the builtin panic function
ident, ok := call.Fun.(*ast.Ident)
if !ok {
return
return true
}

if ident.Name != "panic" {
return
return true
}

// Verify it's the builtin panic, not a user-defined function
if obj := pass.TypesInfo.Uses[ident]; obj != nil {
if _, ok := obj.(*types.Builtin); !ok {
return // Not the builtin panic
return true // Not the builtin panic
}
}

if shouldSkipPanic(pass, call, stack) {
return true
}

pass.ReportRangef(call, "avoid panic in library code; return an error instead")
return true
})

return nil, nil
}

func shouldSkipPanic(pass *analysis.Pass, call *ast.CallExpr, stack []ast.Node) bool {
return isInSyncOnceDoFuncLit(pass, stack) ||
panicMessageStartsWithBUG(pass, call) ||
isInInitFunction(stack) ||
hasDocumentedPanicContract(stack)
}

func isInSyncOnceDoFuncLit(pass *analysis.Pass, stack []ast.Node) bool {
for i := len(stack) - 1; i >= 0; i-- {
funcLit, ok := stack[i].(*ast.FuncLit)
if !ok || i == 0 {
continue
}

call, ok := stack[i-1].(*ast.CallExpr)
if !ok || !containsExpr(call.Args, funcLit) {
continue
}

sel, ok := call.Fun.(*ast.SelectorExpr)
if !ok || sel.Sel.Name != "Do" {
continue
}

if isSyncOnceType(pass.TypesInfo.TypeOf(sel.X)) {
return true
}
}

return false
}

func containsExpr(args []ast.Expr, target ast.Expr) bool {
for _, arg := range args {
if arg == target {
return true
}
}
return false
}

func isSyncOnceType(t types.Type) bool {
if ptr, ok := t.(*types.Pointer); ok {
t = ptr.Elem()
}

named, ok := t.(*types.Named)
if !ok || named.Obj() == nil || named.Obj().Pkg() == nil {
return false
}

return named.Obj().Pkg().Path() == "sync" && named.Obj().Name() == "Once"
}

func panicMessageStartsWithBUG(pass *analysis.Pass, call *ast.CallExpr) bool {
if len(call.Args) == 0 {
return false
}

prefix, ok := stringPrefix(pass, call.Args[0])
if !ok {
return false
}

return strings.HasPrefix(strings.ToUpper(strings.TrimSpace(prefix)), "BUG:")
}

func stringPrefix(pass *analysis.Pass, expr ast.Expr) (string, bool) {
if tv, ok := pass.TypesInfo.Types[expr]; ok && tv.Value != nil && tv.Value.Kind() == constant.String {
return constant.StringVal(tv.Value), true
}

switch e := expr.(type) {
case *ast.BinaryExpr:
if e.Op != token.ADD {
return "", false
}
return stringPrefix(pass, e.X)
case *ast.CallExpr:
if len(e.Args) == 0 {
return "", false
}
return stringPrefix(pass, e.Args[0])
Comment on lines +158 to +162
default:
return "", false
}
}

func isInInitFunction(stack []ast.Node) bool {
decl := enclosingFuncDecl(stack)
return decl != nil && decl.Name != nil && decl.Name.Name == "init"
}

func hasDocumentedPanicContract(stack []ast.Node) bool {
decl := enclosingFuncDecl(stack)
if decl == nil || decl.Doc == nil {
return false
}

doc := strings.ToLower(decl.Doc.Text())
return strings.Contains(doc, "panics on") ||
strings.Contains(doc, "panics if") ||
strings.Contains(doc, "panic on") ||
strings.Contains(doc, "panic if")
}

func enclosingFuncDecl(stack []ast.Node) *ast.FuncDecl {
for i := len(stack) - 1; i >= 0; i-- {
if decl, ok := stack[i].(*ast.FuncDecl); ok {
return decl
}
}
return nil
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package panicinlibrarycode

import "errors"
import (
"errors"
"fmt"
"sync"
)

// bad: panic in a pkg/ package.
func riskyFunction() {
Expand Down Expand Up @@ -28,3 +32,26 @@ func callCustomPanic() {
m := myType{}
m.panic("this is ok") // Should not be flagged
}

var once sync.Once

func allowedSyncOncePanic() {
once.Do(func() {
panic("lazy init failure")
})
}

func allowedBUGPanic() {
panic(fmt.Sprintf("BUG: unreachable: %v", errors.New("boom")))
}

func init() {
panic("startup registration failure")
}

// documentedPreconditionPanics panics if the caller passes an invalid mode.
func documentedPreconditionPanics(mode string) {
if mode == "" {
panic("invalid mode")
}
}
3 changes: 2 additions & 1 deletion pkg/workflow/agentic_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,8 @@ var (
registryInitOnce sync.Once
)

// NewEngineRegistry creates a new engine registry with built-in engines
// NewEngineRegistry creates a new engine registry with built-in engines.
// Panics on invalid built-in engine registration.
func NewEngineRegistry() *EngineRegistry {
agenticEngineLog.Print("Creating new engine registry")

Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/claude_tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ func (e *ClaudeEngine) expandNeutralToolsToClaudeTools(tools map[string]any) map
// user-visible tools map but must be explicitly added to --allowed-tools when
// --permission-mode acceptEdits is in use, because acceptEdits actually enforces the
// allowlist (unlike bypassPermissions which silently ignores it).
// Panics if callers pass a Claude-specific tools section instead of neutral tools.
func (e *ClaudeEngine) computeAllowedClaudeToolsString(tools map[string]any, safeOutputs *SafeOutputsConfig, cacheMemoryConfig *CacheMemoryConfig, mcpScripts *MCPScriptsConfig, sandboxConfig *SandboxConfig) string {
claudeToolsLog.Print("Computing allowed Claude tools string")

Expand Down
2 changes: 2 additions & 0 deletions pkg/workflow/model_aliases.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ type builtinModelAliasesFile struct {
// - "claude" → agent, sonnet-6x, haiku, any
// - "codex" → agent, gpt-5-codex, gpt-5, any
// - "gemini" → agent, gemini-pro, gemini-flash, any
//
// Panics on invalid embedded model_aliases.json data.
func BuiltinModelAliases() map[string][]string {
var data builtinModelAliasesFile
if err := json.Unmarshal(builtinModelAliasesJSON, &data); err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ func ShortenCommand(command string) string {
//
// When seed is empty, the function falls back to crypto/rand — the same behaviour as
// GenerateHeredocDelimiter — so callers that lack a hash continue to work correctly.
// Panics on crypto/rand failure when no seed is provided.
func GenerateHeredocDelimiterFromSeed(name string, seed string) string {
upperName := strings.ToUpper(name)
var tag string
Expand Down