check-safety/authprecedence.go
Alex Dunmow cd88c808b0 initial: standalone check-safety module hoisted from CMS
Static safety/lint runner for the BlockNinja codebase. ~25 invariant
checks across Go and frontend sources. Was at git.dev.alexdunmow.com:block/ninja
in backend/cmd/check-safety/ until the 2026-06-06 consolidation moved
the BlockNinja repos under a shared ~/src/blockninja/ parent.

This repo is the standalone extraction:
- Own go.mod (git.dev.alexdunmow.com/block/check-safety, go 1.26.4)
- Vendored internal/{helpers,theme} from CMS (Go's internal/ rule
  blocks cross-module imports; vendoring is the workaround)
- CLI contract unchanged: `check-safety <target-dir> [--flags]`
- CMS Makefile shells into ../check-safety for safety-check /
  install-safety-checker targets

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-06-06 13:04:02 +08:00

197 lines
5.6 KiB
Go

package main
import (
"bufio"
"os"
"path/filepath"
"strings"
"git.dev.alexdunmow.com/block/check-safety/internal/helpers"
)
type authPrecedenceViolation struct {
file string
line int
cookieLine int
snippet string
}
// checkAuthPrecedence scans Go files for functions that extract auth tokens
// where cookies are checked before Bearer/Authorization headers.
//
// Bearer tokens are explicit per-request credentials. Cookies are ambient
// and may be stale (e.g., set by a prior registration while the caller
// intends to authenticate as a different user via Bearer). If both are
// present, Bearer must win.
//
// The anti-pattern:
//
// cookie, err := r.Cookie("access_token") // cookie read first
// if err == nil { token = cookie.Value }
// if token == "" { // bearer only as fallback
// auth := r.Header.Get("Authorization")
// ...
// }
//
// The correct pattern:
//
// auth := r.Header.Get("Authorization") // bearer first
// if bearer, ok := strings.CutPrefix(auth, "Bearer "); ok {
// token = bearer
// }
// if token == "" { // cookie only as fallback
// cookie, err := r.Cookie(...)
// ...
// }
func checkAuthPrecedence(root string) []authPrecedenceViolation {
var violations []authPrecedenceViolation
_ = filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
if err != nil || info.IsDir() || !strings.HasSuffix(path, ".go") {
return nil
}
if strings.Contains(path, "/vendor/") || strings.HasSuffix(path, "_test.go") {
return nil
}
vs := scanFileAuthPrecedence(path, root)
violations = append(violations, vs...)
return nil
})
return violations
}
// scanFileAuthPrecedence scans a single Go file for the cookie-before-bearer
// anti-pattern. It tracks, per function scope, the first line where a cookie
// is read and the first line where a Bearer/Authorization header is read.
// If the cookie read comes first AND the bearer read is inside a
// "if token == """ guard, it's a violation.
func scanFileAuthPrecedence(path, root string) []authPrecedenceViolation {
f, err := os.Open(path)
if err != nil {
return nil
}
defer helpers.LogDeferredError(nil, "close auth scan file", f.Close, "path", path)
relPath, _ := filepath.Rel(root, path)
var violations []authPrecedenceViolation
scanner := bufio.NewScanner(f)
lineNum := 0
braceDepth := 0
funcStartDepth := -1
// Per-function state
cookieLine := 0
bearerLine := 0
bearerGuarded := false // bearer read is inside "if token == """
resetFunc := func() {
cookieLine = 0
bearerLine = 0
bearerGuarded = false
}
for scanner.Scan() {
lineNum++
line := scanner.Text()
trimmed := strings.TrimSpace(line)
// Track brace depth to detect function boundaries
for _, ch := range line {
switch ch {
case '{':
braceDepth++
case '}':
braceDepth--
}
}
// Detect function start
if strings.HasPrefix(trimmed, "func ") || strings.HasPrefix(trimmed, "func(") {
resetFunc()
funcStartDepth = braceDepth
}
// Detect function end
if funcStartDepth >= 0 && braceDepth < funcStartDepth {
// Function ended — emit violation if cookie came before bearer
if cookieLine > 0 && bearerLine > 0 && cookieLine < bearerLine && bearerGuarded {
violations = append(violations, authPrecedenceViolation{
file: relPath,
line: cookieLine,
cookieLine: cookieLine,
snippet: "cookie checked before Bearer token — Bearer must take precedence over ambient cookies",
})
}
resetFunc()
funcStartDepth = -1
}
// Detect cookie read: r.Cookie(...)
if cookieLine == 0 && (strings.Contains(trimmed, ".Cookie(") && !strings.HasPrefix(trimmed, "//")) {
// Exclude http.SetCookie and cookie-setting lines
if !strings.Contains(trimmed, "SetCookie") && !strings.Contains(trimmed, "http.Cookie{") {
cookieLine = lineNum
}
}
// Detect bearer/authorization read
if bearerLine == 0 && !strings.HasPrefix(trimmed, "//") {
if strings.Contains(trimmed, `"Authorization"`) ||
strings.Contains(trimmed, `"Bearer "`) ||
strings.Contains(trimmed, `"Bearer "`) {
bearerLine = lineNum
// Check if this line or a recent line has a guard like `if token == ""`
// by scanning back up to 3 lines for `token == ""`
bearerGuarded = isBearerGuarded(path, lineNum)
}
}
}
// Handle case where function is the last thing in the file
if cookieLine > 0 && bearerLine > 0 && cookieLine < bearerLine && bearerGuarded {
violations = append(violations, authPrecedenceViolation{
file: relPath,
line: cookieLine,
cookieLine: cookieLine,
snippet: "cookie checked before Bearer token — Bearer must take precedence over ambient cookies",
})
}
return violations
}
// isBearerGuarded checks whether the bearer read around the given line
// is inside a guard like `if token == ""` or `if tok == ""`, meaning
// it only fires when no cookie was found — the anti-pattern.
func isBearerGuarded(path string, bearerLineNum int) bool {
f, err := os.Open(path)
if err != nil {
return false
}
defer helpers.LogDeferredError(nil, "close bearer guard scan file", f.Close, "path", path)
scanner := bufio.NewScanner(f)
lineNum := 0
// Look at lines bearerLineNum-3 through bearerLineNum
for scanner.Scan() {
lineNum++
if lineNum < bearerLineNum-3 {
continue
}
if lineNum > bearerLineNum {
break
}
line := strings.TrimSpace(scanner.Text())
// Common guard patterns: `if token == ""`, `if tok == ""`
if strings.Contains(line, `== ""`) &&
(strings.Contains(line, "token") || strings.Contains(line, "tok ")) {
return true
}
}
return false
}