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
2 changes: 1 addition & 1 deletion docs/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,7 @@ These rules live outside the repository-wide `Change Safety` section in report o
| Defensive programming | `defensive.invalid-state-representable` | warn | Booleans or raw status strings can represent impossible state combinations. |
| Defensive programming | `defensive.null-assumption` | warn | Nullable boundary values are dereferenced without a nil/null guard. |
| Defensive programming | `defensive.integer-overflow` | warn | Arithmetic on count, size, or length input lacks an overflow bound check. |
| Defensive programming | `defensive.sequence-collision-risk` | warn | External ID allocation derives the next value from current count without guarded unique-collision retry. |
| Defensive programming | `defensive.sequence-collision-risk` | warn | External ID allocation derives the next value from current count; bounded unique-collision retry is treated as mitigation, not full resolution. |
| Defensive programming | `defensive.bounds-assumption` | warn | Indexed access assumes collection bounds without a nearby length check. |
| Defensive programming | `defensive.unsafe-default` | warn | A config/env fallback can fail open or disable a safety control. |
| Defensive programming | `defensive.non-exhaustive-branch` | warn | Enum-like state/kind/type branching lacks default or exhaustive handling. |
Expand Down
101 changes: 84 additions & 17 deletions internal/codeguard/checks/quality/quality_defensive.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ const (
)

var (
indexAccessPattern = regexp.MustCompile(`\b([A-Za-z_][\w$]*(?:\.[A-Za-z_][\w$]*)?)\s*\[\s*(?:0|[A-Za-z_][\w$]*)\s*\]`)
indexAccessPattern = regexp.MustCompile(`\b([A-Za-z_][\w$]*(?:\.[A-Za-z_][\w$]*)?)\s*\[\s*([^\]\n]+)\s*\]`)
jsonDecodePattern = regexp.MustCompile(`(?i)(json\.Unmarshal|json\.NewDecoder|JSON\.parse|json\.loads|nlohmann::json::parse|decode_json|parseJson)`)
externalCallPattern = regexp.MustCompile(`(?i)(http\.Get|client\.Do|fetch\s*\(|axios\.|requests\.(get|post|put|delete)|curl_easy_perform|httplib::|http_client)`)
resourceReadPattern = regexp.MustCompile(`(?i)(io\.ReadAll|ReadAll|read_to_string|read_to_end|\.read\s*\(|bodyParser|multer|upload|request\.body|r\.Body)`)
resourceReadPattern = regexp.MustCompile(`(?i)(io\.ReadAll|ReadAll|read_to_string|read_to_end|\.read\s*\(|bodyParser|multer|upload|formData\s*\(|request\.body|r\.Body|findMany\s*\(|findFirst\s*\()`)
unsafeDefaultPattern = regexp.MustCompile(`(?i)(getenv|process\.env|os\.environ|std::getenv|config).*?(default|fallback|\|\||!=|,\s*['"]).*?(true|false|allow|disable|skip|insecure)`)
switchLikePattern = regexp.MustCompile(`(?i)\b(switch|match)\b[^{:\n]*(status|state|kind|type)`)
stateAssignmentPattern = regexp.MustCompile(`(?i)(status|state)\s*(?:=|:=|=>)\s*["']?(paid|active|complete|completed|shipped|deleted|approved)["']?`)
Expand All @@ -40,6 +40,7 @@ var (
resourceNamedCountLimit = regexp.MustCompile(`(?i)\b(?:max|limit|quota|cap)[A-Za-z0-9_]*(?:count|size|length|len|bytes)\b`)
sequenceAllocationLine = regexp.MustCompile(`(?i)\b(?:external[_]?id|next[_]?id|sequence|slug|number)\b.*(?:count|max)\s*\+\s*1|(?:count|max)\s*\+\s*1.*\b(?:external[_]?id|next[_]?id|sequence|slug|number)\b`)
jsonReaderSchemaCall = regexp.MustCompile(`(?i)\b(?:read|parse|decode)Json[A-Za-z0-9_]*\s*\([^)\n,]+,\s*[A-Za-z_$][\w$]*(?:Schema|Validator|Codec|Parser)\b`)
prismaTakePattern = regexp.MustCompile(`(?is)\b(?:findMany|findFirst|findUnique|query|search)\s*\([^)]*\btake\s*:`)
)

func defensiveBoundaryFindings(env support.Context, file string, fn precisionFunction) []core.Finding {
Expand All @@ -58,9 +59,9 @@ func defensiveBoundaryFindings(env support.Context, file string, fn precisionFun
findings = append(findings, precisionWarnFinding(env, defensiveNullAssumptionRuleID, file, line,
"nullable boundary value is dereferenced without a nil/null guard", core.ConfidenceMedium))
}
if line, ok := sequenceCollisionRiskLine(fn, loweredBody); ok {
if line, message, confidence, ok := sequenceCollisionRiskLine(file, fn, loweredBody); ok {
findings = append(findings, precisionWarnFinding(env, defensiveSequenceCollisionRiskRuleID, file, line,
"external ID allocation derives the next value from current count without guarded unique-collision retry", core.ConfidenceMedium))
message, confidence))
}
if line, ok := integerOverflowLine(file, fn, loweredBody); ok {
findings = append(findings, precisionWarnFinding(env, defensiveIntegerOverflowRuleID, file, line,
Expand Down Expand Up @@ -230,7 +231,7 @@ func firstUseLine(fn precisionFunction, name string) int {
}

func integerOverflowLine(file string, fn precisionFunction, loweredBody string) (int, bool) {
if isUIRenderArithmeticContext(file, fn, loweredBody) {
if isUIRenderArithmeticContext(file, fn, loweredBody) || isSeedOrScriptSourcePath(file) {
return 0, false
}
if sequenceAllocationArithmetic(loweredBody) || metricStatArithmeticContext(fn, loweredBody) || dateCountFormattingContext(fn, loweredBody) {
Expand All @@ -248,11 +249,21 @@ func integerOverflowLine(file string, fn precisionFunction, loweredBody string)
return 0, false
}

func sequenceCollisionRiskLine(fn precisionFunction, loweredBody string) (int, bool) {
if !sequenceAllocationArithmetic(loweredBody) || guardedSequenceCollisionRetry(loweredBody) {
return 0, false
func sequenceCollisionRiskLine(file string, fn precisionFunction, loweredBody string) (int, string, string, bool) {
if isSeedOrScriptSourcePath(file) || !sequenceAllocationArithmetic(loweredBody) {
return 0, "", core.ConfidenceLow, false
}
line := firstSequenceAllocationLine(fn)
if guardedSequenceCollisionRetry(loweredBody) {
return line,
"external ID allocation is protected by bounded unique-collision retry, but count-derived IDs remain architectural debt; prefer a database sequence or transactional allocator",
core.ConfidenceLow,
true
}
return firstSequenceAllocationLine(fn), true
return line,
"external ID allocation derives the next value from current count; use a database sequence, UUID, or transactional allocator instead of count-based generation",
core.ConfidenceMedium,
true
}

func sequenceAllocationArithmetic(loweredBody string) bool {
Expand All @@ -273,7 +284,7 @@ func firstSequenceAllocationLine(fn precisionFunction) int {
}

func guardedSequenceCollisionRetry(loweredBody string) bool {
if containsAny(loweredBody, []string{"withexternalidretry", "with_external_id_retry"}) &&
if containsAny(loweredBody, []string{"withexternalidretry", "with_external_id_retry", "externalidretry", "external_id_retry"}) &&
containsAny(loweredBody, []string{"p2002", "unique", "collision", "externalid", "external_id"}) {
return true
}
Expand All @@ -296,12 +307,13 @@ func metricStatArithmeticContext(fn precisionFunction, loweredBody string) bool

func dateCountFormattingContext(fn precisionFunction, loweredBody string) bool {
loweredName := strings.ToLower(fn.Name)
if !containsAny(loweredName, []string{"format", "display", "label", "render", "summary", "calendar", "date", "time"}) {
if !containsAny(loweredName, []string{"format", "display", "label", "render", "summary", "calendar", "date", "time", "bucket", "group"}) {
return false
}
return containsAny(loweredBody, []string{
"date", "time", "calendar", "duration", "intl.", "datetimeformat", "formatdistance",
"formatrelative", "plural", "label", "title", "subtitle", "`${", " + \"", " + '",
"formatrelative", "plural", "label", "title", "subtitle", "bucket", "startof", "endof",
"adddays", "subdays", "dayjs", "date-fns", "`${", " + \"", " + '",
})
}

Expand All @@ -321,6 +333,24 @@ func isUIRenderArithmeticContext(file string, fn precisionFunction, loweredBody
})
}

func isSeedOrScriptSourcePath(file string) bool {
normalized := strings.ToLower(strings.ReplaceAll(file, "\\", "/"))
base := normalized
if slash := strings.LastIndex(base, "/"); slash >= 0 {
base = base[slash+1:]
}
return strings.Contains(normalized, "/scripts/") ||
strings.Contains(normalized, "/script/") ||
strings.Contains(normalized, "/seed") ||
strings.Contains(normalized, "/seeds/") ||
strings.Contains(normalized, "/backfill") ||
strings.Contains(normalized, "/import") ||
strings.HasPrefix(base, "seed") ||
strings.HasPrefix(base, "backfill") ||
strings.HasPrefix(base, "import") ||
strings.HasPrefix(base, "cleanup")
}

func boundsAssumptionLine(fn precisionFunction, loweredBody string) (int, bool) {
if containsAny(loweredBody, []string{"len(", ".length", ".size()", "empty()", "bounds", "range", "count >"}) {
return 0, false
Expand All @@ -334,16 +364,38 @@ func boundsAssumptionLine(fn precisionFunction, loweredBody string) (int, bool)
if match == nil {
continue
}
if nearbyBoundsGuard(fn.Statements, idx, match[1]) {
if !indexExpressionLooksSequenceAccess(match[1], match[2], raw) {
continue
}
if indexAccessPattern.MatchString(raw) {
return statement.Line, true
if nearbyBoundsGuard(fn.Statements, idx, match[1]) {
continue
}
return statement.Line, true
}
return 0, false
}

func indexExpressionLooksSequenceAccess(target string, key string, raw string) bool {
loweredTarget := strings.ToLower(strings.TrimSpace(target))
loweredKey := strings.ToLower(strings.TrimSpace(strings.Trim(key, `"'`)))
loweredRaw := strings.ToLower(raw)
if strings.Contains(loweredTarget, "process.env") || strings.Contains(loweredTarget, "env") ||
strings.Contains(loweredTarget, "map") || strings.Contains(loweredTarget, "dict") || strings.Contains(loweredTarget, "lookup") {
return false
}
if containsAny(loweredRaw, []string{"record<", "map<", "dictionary", "dict", "object.", "hasown", " in ", ".has("}) {
return false
}
if regexp.MustCompile(`^\d+$`).MatchString(loweredKey) {
return true
}
if containsAny(loweredKey, []string{"index", "idx", "offset", "position", "pos", "i", "j", "n"}) {
return true
}
return containsAny(loweredTarget, []string{"array", "list", "slice", "items", "rows", "columns", "chars", "parts", "tokens", "segments", "lines", "values"}) &&
!containsAny(loweredKey, []string{"name", "key", "id", "type", "status", "field"})
}

func nearbyBoundsGuard(statements []support.ParsedStatement, idx int, target string) bool {
target = strings.ToLower(strings.TrimSpace(strings.Split(target, ".")[0]))
if target == "" {
Expand Down Expand Up @@ -427,17 +479,21 @@ func missingResourceLimitLine(fn precisionFunction, loweredBody string) (int, bo
if boundedReadByteLengthCheck(loweredBody) {
return 0, false
}
if preFormDataContentLengthHelperGuard(loweredBody) {
return 0, false
}
return firstPatternLine(fn, resourceReadPattern), true
}

func resourceLimitProofPattern(loweredBody string) bool {
if containsAny(loweredBody, []string{
"limitreader", "maxbytes", "max_bytes", "content-length", "contentlength",
"limit(", "take(", "slice(", ".slice(", "buffer_size", "quota",
"limit(", "take(", "take:", ".take", "slice(", ".slice(", "buffer_size", "quota",
"maxresults", "max_results", "page_size", "pagesize",
}) {
return true
}
return resourceCountGuard.MatchString(loweredBody) || resourceNamedCountLimit.MatchString(loweredBody)
return resourceCountGuard.MatchString(loweredBody) || resourceNamedCountLimit.MatchString(loweredBody) || prismaTakePattern.MatchString(loweredBody)
}

func uploadValidationHelperPattern(loweredBody string) bool {
Expand All @@ -452,6 +508,17 @@ func boundedReadByteLengthCheck(loweredBody string) bool {
return containsAny(loweredBody, []string{"bytelength", "byte_length", ".length > max", ".length > limit", "buffer.length", "bytes.length"})
}

func preFormDataContentLengthHelperGuard(loweredBody string) bool {
if !strings.Contains(loweredBody, "formdata") {
return false
}
return containsAny(loweredBody, []string{
"assertcontentlength", "ensurecontentlength", "validatecontentlength", "guardcontentlength",
"assertrequestsize", "ensurerequestsize", "validaterequestsize", "guardrequestsize",
"assertuploadsize", "ensureuploadsize", "validateuploadsize", "guarduploadsize",
})
}

func invalidStateTransitionLine(fn precisionFunction, loweredBody string) (int, bool) {
if !stateAssignmentPattern.MatchString(functionRawBody(fn)) {
return 0, false
Expand Down
21 changes: 21 additions & 0 deletions internal/codeguard/checks/quality/quality_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ func cleanupIgnoredLine(statements []support.ParsedStatement) (int, bool) {
}

func partialFailureHiddenLine(fn precisionFunction, loweredBody string) (int, bool) {
if partialFailureSurfacedInResult(loweredBody) {
return 0, false
}
if strings.Contains(loweredBody, "allsettled") && !containsAny(loweredBody, []string{"rejected", "throw", "return err", "return error"}) {
return fn.StartLine, true
}
Expand All @@ -205,6 +208,24 @@ func partialFailureHiddenLine(fn precisionFunction, loweredBody string) (int, bo
return 0, false
}

func partialFailureSurfacedInResult(loweredBody string) bool {
if !containsAny(loweredBody, []string{"diagnostic", "diagnostics", "errors", "failures", "warnings"}) {
return false
}
recordsFailure := containsAny(loweredBody, []string{
"diagnostics.push", "errors.push", "failures.push", "warnings.push",
"append(diagnostics", "append(errors", "append(failures", "append(warnings",
"diagnostics = append", "errors = append", "failures = append", "warnings = append",
})
if !recordsFailure {
return false
}
return containsAny(loweredBody, []string{
"return {", "return result", "diagnostics:", "errors:", "failures:", "warnings:",
"diagnostics,", "errors,", "failures,", "warnings,",
})
}

func fallbackHidesCorruptionLine(fn precisionFunction, loweredBody string) (int, bool) {
if !containsAny(loweredBody, []string{"json", "parse", "deserialize", "unmarshal", "decode", "validate", "corrupt"}) {
return 0, false
Expand Down
2 changes: 1 addition & 1 deletion internal/codeguard/checks/quality/quality_precision.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ var (
"misc": {}, "stuff": {}, "value": {}, "values": {},
}
queryFunctionPrefixPattern = regexp.MustCompile(`^(get|find|list|load|read|lookup|fetch|is|has|can|should|compute|calculate|build|format|parse)`)
mutatingCallPattern = regexp.MustCompile(`(?i)(^|[.>:\-_])(add|append|assign|clear|create|delete|emit|insert|mutate|persist|pop|publish|push|push_back|remove|reverse|save|send|set|sort|splice|store|update|upsert|write)([A-Z_:\-.]|$)`)
mutatingCallPattern = regexp.MustCompile(`(?i)(^|[.>:\-_])(add|allocate|append|assign|clear|create|delete|emit|insert|mutate|persist|pop|publish|push|push_back|remove|reverse|save|send|set|sort|splice|store|update|upsert|write)([A-Z_:\-.]|$)`)
lowLevelOperationPattern = regexp.MustCompile(`(?i)(\bsql\.|\.query\(|\.exec\(|\bhttp\.|\bfetch\(|\baxios\.|\brequests\.|\bjson\.|\bJSON\.|\bos\.Getenv\b|\bprocess\.env\b|\bfs\.|#include\b)`)
primitiveTypePattern = regexp.MustCompile(`(?i)\b(string|str|int|int64|float|float64|double|decimal|number|boolean|bool|char|long|short)\b`)
domainPrimitiveNamePattern = regexp.MustCompile(`(?i)(id|status|state|type|kind|currency|amount|price|email|phone|country|role|permission|tenant|account|customer|order)`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const (
)

var (
commandFunctionPrefixPattern = regexp.MustCompile(`^(add|append|assign|cancel|clear|close|create|delete|disable|emit|enable|insert|mutate|notify|open|persist|publish|record|remove|reset|save|send|set|store|submit|toggle|update|upsert|upload|write)`)
commandFunctionPrefixPattern = regexp.MustCompile(`^(add|allocate|append|assign|cancel|clear|close|create|delete|disable|emit|enable|insert|mutate|notify|open|persist|publish|record|remove|reset|save|send|set|store|submit|toggle|update|upsert|upload|write)`)
readCallPattern = regexp.MustCompile(`(?i)(^|[.>:\-_])(count|fetch|find|get|list|load|lookup|query|read|select|search)([A-Z_:\-.]|$)`)
identifierTokenPattern = regexp.MustCompile(`[A-Za-z_$][A-Za-z0-9_$]*`)
infraNamePattern = regexp.MustCompile(`(?i)(sql|http|redis|kafka|grpc|graphql|mongo|s3|dynamo|postgres|mysql|elastic|orm)`)
Expand Down Expand Up @@ -330,7 +330,7 @@ func explicitMutationName(name string) bool {
}

func inconsistentReturnContract(fn precisionFunction) bool {
if nextResponseNullableGuardHelper(fn) {
if nextResponseNullableGuardHelper(fn) || nullableParserLookupContract(fn) {
return false
}
returns := returnCategories(fn.Body)
Expand All @@ -351,6 +351,21 @@ func nextResponseNullableGuardHelper(fn precisionFunction) bool {
return strings.Contains(body, "return null") && hasNextResponseBody
}

func nullableParserLookupContract(fn precisionFunction) bool {
loweredName := strings.ToLower(strings.Trim(fn.Name, "_$"))
if !regexp.MustCompile(`^(as|exists|extract|find|get|lookup|parse|read|resolve|to)`).MatchString(loweredName) {
return false
}
body := strings.ToLower(fn.Body)
if loweredName == "exists" || strings.HasPrefix(loweredName, "exists") {
return containsAny(body, []string{"return false", "return true"})
}
signature := strings.ToLower(fn.Signature)
hasNullableReturnEvidence := containsAny(signature, []string{"| null", "|null", "null", "undefined", "optional"}) ||
containsAny(body, []string{"return null", "return undefined", "return none", "return nil"})
return hasNullableReturnEvidence && containsAny(body, []string{"return null", "return undefined", "return none", "return nil"})
}

type returnShapeCounts struct {
total int
empty bool
Expand Down Expand Up @@ -387,7 +402,7 @@ func returnCategories(body string) returnShapeCounts {
func isEmptyReturnExpr(expr string) bool {
expr = strings.TrimSpace(strings.TrimSuffix(expr, ";"))
switch strings.ToLower(expr) {
case "", "nil", "none", "null", "undefined", "false":
case "", "nil", "none", "null", "undefined":
return true
default:
return strings.HasPrefix(expr, "nil,") || strings.HasPrefix(expr, "none,") || strings.HasPrefix(expr, "null,")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var qualityErrorDefensiveFixTemplates = map[string]core.FixTemplate{
"defensive.invalid-state-representable": {Kind: guided, Text: "Replace boolean combinations/raw strings with an enum, tagged union, or state machine that encodes valid states."},
"defensive.null-assumption": {Kind: guided, Text: "Guard nil/null/None/optional values before dereference, or make the boundary type non-nullable."},
"defensive.integer-overflow": {Kind: guided, Text: "Guard count/size arithmetic before multiplication, addition, shifts, or allocation sizing."},
"defensive.sequence-collision-risk": {Kind: guided, Text: "Replace count-plus-one external IDs with database sequences/UUIDs, or wrap allocation in a bounded unique-collision retry."},
"defensive.sequence-collision-risk": {Kind: guided, Text: "Replace count-plus-one external IDs with database sequences, UUIDs, or a transactional allocator. A bounded P2002/unique-collision retry mitigates collisions but should still be treated as architecture debt."},
"defensive.bounds-assumption": {Kind: guided, Text: "Check length/existence before indexing, or use a safe lookup API."},
"defensive.unsafe-default": {Kind: guided, Text: "Make security/safety defaults fail closed and require explicit opt-out for unsafe behavior."},
"defensive.non-exhaustive-branch": {Kind: guided, Text: "Add an explicit default/unreachable branch or exhaustive assertion for enum-like state switches."},
Expand Down
Loading
Loading