diff --git a/pkg/analysis/passes/coderules/coderules_test.go b/pkg/analysis/passes/coderules/coderules_test.go index 236ed5f7..b5f9bdb0 100644 --- a/pkg/analysis/passes/coderules/coderules_test.go +++ b/pkg/analysis/passes/coderules/coderules_test.go @@ -317,6 +317,306 @@ func TestOldReactInternals(t *testing.T) { ) } +func TestOutdatedSqldsVersion(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "outdated-sqlds-bad"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 2) + for i := range interceptor.Diagnostics { + require.Equal( + t, + "Outdated sqlds version detected (v1 or v2). Use sqlds/v3 or sqlds/v4 which have updated signatures that allow passing context.Context for forward compatibility.", + interceptor.Diagnostics[i].Title, + ) + require.Equal(t, analysis.Warning, interceptor.Diagnostics[i].Severity) + require.Equal( + t, + "code-rules-outdated-sqlds-version", + interceptor.Diagnostics[i].Name, + ) + } +} + +func TestOutdatedSqldsVersionGood(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "outdated-sqlds-good"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 0) +} + +func TestNativeBrowserDialogs(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "native-browser-dialogs-bad"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 4) + for i := range interceptor.Diagnostics { + require.Equal( + t, + "Native browser dialogs (alert, confirm, prompt) are not permitted. Use Grafana UI components (Modal, ConfirmModal) instead.", + interceptor.Diagnostics[i].Title, + ) + require.Equal(t, analysis.Error, interceptor.Diagnostics[i].Severity) + require.Equal( + t, + "code-rules-native-browser-dialogs", + interceptor.Diagnostics[i].Name, + ) + } +} + +func TestFmtPrintLogging(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "fmt-print-logging"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 3) + for i := range interceptor.Diagnostics { + require.Equal( + t, + "Use the logger provided by the Grafana plugin SDK (github.com/grafana/grafana-plugin-sdk-go/backend) instead of fmt.Println/fmt.Print/fmt.Printf for proper log management and integration with Grafana's logging system.", + interceptor.Diagnostics[i].Title, + ) + require.Equal(t, analysis.Error, interceptor.Diagnostics[i].Severity) + require.Equal( + t, + "code-rules-fmt-print-logging", + interceptor.Diagnostics[i].Name, + ) + } +} + +func TestWindowOpenWithoutNoopener(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "window-open-bad"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 3) + for i := range interceptor.Diagnostics { + require.Equal( + t, + "window.open() called without 'noopener,noreferrer' in the features parameter. This creates a tab nabbing vulnerability. Use window.open(url, target, 'noopener,noreferrer').", + interceptor.Diagnostics[i].Title, + ) + require.Equal(t, analysis.Error, interceptor.Diagnostics[i].Severity) + require.Equal( + t, + "code-rules-window-open-without-noopener", + interceptor.Diagnostics[i].Name, + ) + } +} + +func TestWindowOpenWithNoopenerGood(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "window-open-good"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 0) +} + +func TestDeprecatedGfFormCSSClasses(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "deprecated-gf-form-bad"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Greater(t, len(interceptor.Diagnostics), 0) + for i := range interceptor.Diagnostics { + require.Equal( + t, + "Deprecated Grafana CSS class name detected (gf-form*). Use @grafana/ui components instead of legacy CSS classes.", + interceptor.Diagnostics[i].Title, + ) + require.Equal(t, analysis.Warning, interceptor.Diagnostics[i].Severity) + require.Equal( + t, + "code-rules-deprecated-gf-form-css-classes", + interceptor.Diagnostics[i].Name, + ) + } +} + +func TestDirectWindowLocationAccess(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "direct-window-location-bad"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 4) + for i := range interceptor.Diagnostics { + require.Equal( + t, + "Direct access to window.location is not permitted. Use locationService from @grafana/runtime instead.", + interceptor.Diagnostics[i].Title, + ) + require.Equal(t, analysis.Warning, interceptor.Diagnostics[i].Severity) + require.Equal( + t, + "code-rules-direct-window-location-access", + interceptor.Diagnostics[i].Name, + ) + } +} + +func TestDirectWindowLocationAccessGood(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "direct-window-location-good"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 0) +} + +func TestTsIgnoreSuppress(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "ts-ignore-bad"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 2) + for i := range interceptor.Diagnostics { + require.Equal( + t, + "Avoid using @ts-ignore or @ts-expect-error to suppress TypeScript errors. Fix TypeScript errors properly so issues are caught during compilation rather than at runtime.", + interceptor.Diagnostics[i].Title, + ) + require.Equal(t, analysis.Warning, interceptor.Diagnostics[i].Severity) + require.Equal( + t, + "code-rules-ts-ignore-suppress", + interceptor.Diagnostics[i].Name, + ) + } +} + +func TestTsIgnoreSuppressGood(t *testing.T) { + if !isSemgrepInstalled() { + t.Skip("semgrep not installed, skipping test") + return + } + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]any{ + sourcecode.Analyzer: filepath.Join("testdata", "ts-ignore-good"), + }, + Report: interceptor.ReportInterceptor(), + } + + _, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Len(t, interceptor.Diagnostics, 0) +} + func TestNoDirectCSSImports(t *testing.T) { if !isSemgrepInstalled() { t.Skip("semgrep not installed, skipping test") diff --git a/pkg/analysis/passes/coderules/semgrep-rules.yaml b/pkg/analysis/passes/coderules/semgrep-rules.yaml index a5d4f917..3e017c53 100644 --- a/pkg/analysis/passes/coderules/semgrep-rules.yaml +++ b/pkg/analysis/passes/coderules/semgrep-rules.yaml @@ -111,6 +111,14 @@ rules: languages: [go] severity: ERROR + - id: outdated-sqlds-version + pattern-either: + - pattern-regex: '"github\.com/grafana/sqlds"' + - pattern-regex: '"github\.com/grafana/sqlds/v2' + message: "Outdated sqlds version detected (v1 or v2). Use sqlds/v3 or sqlds/v4 which have updated signatures that allow passing context.Context for forward compatibility." + languages: [go] + severity: WARNING + - id: console-logging pattern-either: - pattern: console.log(...) @@ -194,6 +202,114 @@ rules: languages: [javascript, typescript] severity: WARNING + - id: native-browser-dialogs + pattern-either: + - pattern: alert(...) + - pattern: window.alert(...) + - pattern: window.confirm(...) + - pattern: window.prompt(...) + paths: + include: + - "src/**/*.ts" + - "src/**/*.tsx" + exclude: + - "*.spec.ts" + - "*.spec.tsx" + - "*.test.ts" + - "*.test.tsx" + - "*.js" + message: "Native browser dialogs (alert, confirm, prompt) are not permitted. Use Grafana UI components (Modal, ConfirmModal) instead." + languages: [javascript, typescript] + severity: ERROR + + - id: fmt-print-logging + pattern-either: + - pattern: fmt.Println(...) + - pattern: fmt.Print(...) + - pattern: fmt.Printf(...) + message: "Use the logger provided by the Grafana plugin SDK (github.com/grafana/grafana-plugin-sdk-go/backend) instead of fmt.Println/fmt.Print/fmt.Printf for proper log management and integration with Grafana's logging system." + languages: [go] + severity: ERROR + + - id: window-open-without-noopener + pattern-either: + - patterns: + - pattern: window.open($URL) + - patterns: + - pattern: window.open($URL, $TARGET) + - patterns: + - pattern: window.open($URL, $TARGET, $FEATURES) + - metavariable-regex: + metavariable: $FEATURES + regex: ^(?!.*noopener.*noreferrer|.*noreferrer.*noopener) + paths: + include: + - "src/**/*.ts" + - "src/**/*.tsx" + exclude: + - "*.spec.ts" + - "*.spec.tsx" + - "*.test.ts" + - "*.test.tsx" + - "*.js" + message: "window.open() called without 'noopener,noreferrer' in the features parameter. This creates a tab nabbing vulnerability. Use window.open(url, target, 'noopener,noreferrer')." + languages: [javascript, typescript] + severity: ERROR + + - id: deprecated-gf-form-css-classes + pattern-either: + - pattern-regex: gf-form + paths: + include: + - "src/**/*.ts" + - "src/**/*.tsx" + exclude: + - "*.spec.ts" + - "*.spec.tsx" + - "*.test.ts" + - "*.test.tsx" + - "*.js" + message: "Deprecated Grafana CSS class name detected (gf-form*). Use @grafana/ui components instead of legacy CSS classes." + languages: [javascript, typescript] + severity: WARNING + + - id: direct-window-location-access + pattern-either: + - pattern: window.location.search + - pattern: window.location.href + - pattern: new URLSearchParams(window.location.search) + paths: + include: + - "src/**/*.ts" + - "src/**/*.tsx" + exclude: + - "*.spec.ts" + - "*.spec.tsx" + - "*.test.ts" + - "*.test.tsx" + - "*.js" + message: "Direct access to window.location is not permitted. Use locationService from @grafana/runtime instead." + languages: [javascript, typescript] + severity: WARNING + + - id: ts-ignore-suppress + pattern-either: + - pattern-regex: "@ts-ignore" + - pattern-regex: "@ts-expect-error" + paths: + include: + - "src/**/*.ts" + - "src/**/*.tsx" + exclude: + - "*.spec.ts" + - "*.spec.tsx" + - "*.test.ts" + - "*.test.tsx" + - "*.js" + message: "Avoid using @ts-ignore or @ts-expect-error to suppress TypeScript errors. Fix TypeScript errors properly so issues are caught during compilation rather than at runtime." + languages: [typescript] + severity: WARNING + - id: no-direct-css-imports pattern-either: - pattern-regex: import\s+['"].*\.(css|scss|less)['"] diff --git a/pkg/analysis/passes/coderules/testdata/access-env-allowed/main.go b/pkg/analysis/passes/coderules/testdata/access-env-allowed/main.go index 97e3c7f9..67736481 100644 --- a/pkg/analysis/passes/coderules/testdata/access-env-allowed/main.go +++ b/pkg/analysis/passes/coderules/testdata/access-env-allowed/main.go @@ -1,7 +1,6 @@ package donotinclude import ( - "fmt" "os" ) @@ -21,10 +20,10 @@ func DoNotInclude() { // GF_PLUGIN are allowed env := os.Getenv("GF_PLUGIN_ALLOWED_ENV") - fmt.Println(env) + _ = env // NOT allowed for _, e := range os.Environ() { - fmt.Println(e) + _ = e } } diff --git a/pkg/analysis/passes/coderules/testdata/access-env/main.go b/pkg/analysis/passes/coderules/testdata/access-env/main.go index b8c8e366..c4f0e59f 100644 --- a/pkg/analysis/passes/coderules/testdata/access-env/main.go +++ b/pkg/analysis/passes/coderules/testdata/access-env/main.go @@ -9,9 +9,9 @@ func DoNotInclude() { panic("This function should never be included in the binary.") env := os.Getenv("DO_NOT_INCLUDE") - fmt.Println(env) + log.Info(env) for _, e := range os.Environ() { - fmt.Println(e) + log.Info(e) } } diff --git a/pkg/analysis/passes/coderules/testdata/deprecated-gf-form-bad/src/index.ts b/pkg/analysis/passes/coderules/testdata/deprecated-gf-form-bad/src/index.ts new file mode 100644 index 00000000..93c19809 --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/deprecated-gf-form-bad/src/index.ts @@ -0,0 +1,10 @@ +import React from 'react'; + +export const MyComponent = () => { + return
+ +
+ +
+
; +}; diff --git a/pkg/analysis/passes/coderules/testdata/direct-window-location-bad/src/index.ts b/pkg/analysis/passes/coderules/testdata/direct-window-location-bad/src/index.ts new file mode 100644 index 00000000..f5c16c00 --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/direct-window-location-bad/src/index.ts @@ -0,0 +1,5 @@ +function getParams() { + const search = window.location.search; + const href = window.location.href; + const params = new URLSearchParams(window.location.search); +} diff --git a/pkg/analysis/passes/coderules/testdata/direct-window-location-good/src/index.ts b/pkg/analysis/passes/coderules/testdata/direct-window-location-good/src/index.ts new file mode 100644 index 00000000..df7c6d57 --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/direct-window-location-good/src/index.ts @@ -0,0 +1,6 @@ +import { locationService } from '@grafana/runtime'; + +function getParams() { + const search = locationService.getSearch(); + const location = locationService.getLocation(); +} diff --git a/pkg/analysis/passes/coderules/testdata/fmt-print-logging/main.go b/pkg/analysis/passes/coderules/testdata/fmt-print-logging/main.go new file mode 100644 index 00000000..0f432217 --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/fmt-print-logging/main.go @@ -0,0 +1,9 @@ +package main + +import "fmt" + +func main() { + fmt.Println("hello") + fmt.Print("hello") + fmt.Printf("hello %s", "world") +} diff --git a/pkg/analysis/passes/coderules/testdata/native-browser-dialogs-bad/src/index.ts b/pkg/analysis/passes/coderules/testdata/native-browser-dialogs-bad/src/index.ts new file mode 100644 index 00000000..27a98f62 --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/native-browser-dialogs-bad/src/index.ts @@ -0,0 +1,6 @@ +function test() { + alert("hello"); + window.alert("hello"); + window.confirm("are you sure?"); + window.prompt("enter your name"); +} diff --git a/pkg/analysis/passes/coderules/testdata/outdated-sqlds-bad/main.go b/pkg/analysis/passes/coderules/testdata/outdated-sqlds-bad/main.go new file mode 100644 index 00000000..4535ff28 --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/outdated-sqlds-bad/main.go @@ -0,0 +1,11 @@ +package main + +import ( + "github.com/grafana/sqlds" + "github.com/grafana/sqlds/v2" +) + +func main() { + _ = sqlds.Driver{} + _ = v2.Driver{} +} diff --git a/pkg/analysis/passes/coderules/testdata/outdated-sqlds-good/main.go b/pkg/analysis/passes/coderules/testdata/outdated-sqlds-good/main.go new file mode 100644 index 00000000..643d5a76 --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/outdated-sqlds-good/main.go @@ -0,0 +1,9 @@ +package main + +import ( + "github.com/grafana/sqlds/v3" +) + +func main() { + _ = v3.Driver{} +} diff --git a/pkg/analysis/passes/coderules/testdata/ts-ignore-bad/src/index.ts b/pkg/analysis/passes/coderules/testdata/ts-ignore-bad/src/index.ts new file mode 100644 index 00000000..5ff55a00 --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/ts-ignore-bad/src/index.ts @@ -0,0 +1,5 @@ +// @ts-ignore +const x: number = "not a number"; + +// @ts-expect-error +const y: string = 123; diff --git a/pkg/analysis/passes/coderules/testdata/ts-ignore-good/src/index.ts b/pkg/analysis/passes/coderules/testdata/ts-ignore-good/src/index.ts new file mode 100644 index 00000000..25106eb3 --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/ts-ignore-good/src/index.ts @@ -0,0 +1,2 @@ +const x: number = 42; +const y: string = "hello"; diff --git a/pkg/analysis/passes/coderules/testdata/window-open-bad/src/index.ts b/pkg/analysis/passes/coderules/testdata/window-open-bad/src/index.ts new file mode 100644 index 00000000..9f3ddd1b --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/window-open-bad/src/index.ts @@ -0,0 +1,5 @@ +function test() { + window.open("https://example.com"); + window.open("https://example.com", "_blank"); + window.open("https://example.com", "_blank", "width=800,height=600"); +} diff --git a/pkg/analysis/passes/coderules/testdata/window-open-good/src/index.ts b/pkg/analysis/passes/coderules/testdata/window-open-good/src/index.ts new file mode 100644 index 00000000..190c00fc --- /dev/null +++ b/pkg/analysis/passes/coderules/testdata/window-open-good/src/index.ts @@ -0,0 +1,5 @@ +function test() { + window.open("https://example.com", "_blank", "noopener,noreferrer"); + window.open("https://example.com", "_blank", "noreferrer,noopener"); + window.open("https://example.com", "_blank", "noopener,noreferrer,width=800"); +} diff --git a/pkg/analysis/passes/llmreview/llmreview.go b/pkg/analysis/passes/llmreview/llmreview.go index 9b226a11..5e15a1fb 100644 --- a/pkg/analysis/passes/llmreview/llmreview.go +++ b/pkg/analysis/passes/llmreview/llmreview.go @@ -31,6 +31,7 @@ var geminiKey = os.Getenv("GEMINI_API_KEY") var ( llmIssueFound = &analysis.Rule{Name: "llm-issue-found", Severity: analysis.SuspectedProblem} + llmWarningFound = &analysis.Rule{Name: "llm-warning-found", Severity: analysis.Warning} llmReviewSkipped = &analysis.Rule{ Name: "llm-review-skipped", Severity: analysis.SuspectedProblem, @@ -116,6 +117,50 @@ var Questions = []llmvalidate.LLMQuestion{ Question: "Only for go/golang code: Does this code create HTTP clients without using github.com/grafana/grafana-plugin-sdk-go/backend/httpclient? (Look for direct creation of http.Client{}, http.NewRequest, calls to third-party NewClient/NewHTTPClient functions that don't accept or use the SDK's httpclient, or any other HTTP client initialization that doesn't use github.com/grafana/grafana-plugin-sdk-go/backend/httpclient. The httpclient from github.com/grafana/grafana-plugin-sdk-go/backend/httpclient should be used directly or passed to the HTTP client being created. This includes cases where third-party libraries create HTTP clients internally - those libraries should accept the SDK's httpclient as a parameter). Provide the specific code snippet if found.", ExpectedAnswer: false, }, + { + Question: "Does this code log sensitive information such as credentials, tokens, passwords, API keys, request bodies, or full request/response objects at INFO level or higher? (These should use DEBUG level only). Provide the specific code snippet if found.", + ExpectedAnswer: false, + }, + { + Question: "Only for go/golang code: Does this code use incorrect log formatting? (Look for patterns like `log.Info(\"message\", err)` instead of the correct `log.Info(\"message\", \"error\", err)` with key-value pairs, or logging that produces 'EXTRA_VALUE_AT_END' in output). Provide the specific code snippet if found.", + ExpectedAnswer: false, + }, + { + Question: "Does this code render user-supplied or dynamic content as HTML without sanitization? (Look for dangerouslySetInnerHTML without DOMPurify, d3.html() with user data, innerHTML assignments, or markdown-it with html:true without sanitization). Provide the specific code snippet if found.", + ExpectedAnswer: false, + }, + { + Question: "Only for go/golang code: Does this code use panic() for error handling instead of returning errors properly? (panic should only be used for truly unrecoverable situations, not for regular error handling). Provide the specific code snippet if found.", + ExpectedAnswer: false, + }, + { + Question: "Does this code use localStorage or sessionStorage with generic key names (not namespaced with the plugin ID) that could conflict with Grafana core or other plugins? Provide the specific code snippet if found.", + ExpectedAnswer: false, + }, + { + Question: "For plugins that have multiple plugin.json files: Are the grafanaDependency values inconsistent across them? (The grafanaDependency property must be consistent across all plugins). Provide the specific plugin.json files and their grafanaDependency values if found to be inconsistent.", + ExpectedAnswer: false, + }, + { + Question: "Only for go/golang code: Does this code access attributes or methods of a returned value before checking if it is nil? (Code that accesses returned values must be moved after error/nil checks to prevent nil pointer dereference crashes. For example, if a function returns `(req *Request, err error)`, code accessing `req` should be after checking `if err != nil` or `if req == nil`). Provide the specific code snippet showing the unsafe access if found.", + ExpectedAnswer: false, + }, +} + +// OptionalQuestions are non-blocking suggestions that can be addressed in future versions +var OptionalQuestions = []llmvalidate.LLMQuestion{ + { + Question: "Only for go/golang code: In QueryData or CheckHealth handlers, does this code create a new context (context.Background() or context.TODO()) instead of using/forwarding the context received from the request? Provide the specific code snippet if found.", + ExpectedAnswer: false, + }, + { + Question: "Does the src/README.md file contain installation instructions for the plugin? (Installation instructions should be removed from src/README.md as this information will be included in the Grafana catalog once the plugin is published and may cause confusion). Provide the specific section or content if found.", + ExpectedAnswer: false, + }, + { + Question: "Does this code specify exact pixel values, font sizes, margins, or other hardcoded CSS values instead of using Grafana's emotion theme abstractions? (Rather than specifying exact pixels, font sizes, etc., it's recommended to use the abstractions defined in Grafana's emotion theme which is exposed by `@grafana/data`. This ensures consistency with Grafana's design system and better maintainability). Provide the specific code snippet showing hardcoded CSS values if found.", + ExpectedAnswer: false, + }, } func run(pass *analysis.Pass) (any, error) { @@ -160,36 +205,17 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } - var answers []llmvalidate.LLMAnswer - answers, err = llmClient.AskLLMAboutCode(sourceCodeDir, Questions, []string{"src", "pkg"}) + // Process mandatory questions (blocking issues) + var mandatoryAnswers []llmvalidate.LLMAnswer + mandatoryAnswers, err = llmClient.AskLLMAboutCode(sourceCodeDir, Questions, []string{"src", "pkg"}) if err != nil { - logme.DebugFln("Error getting answers from Gemini LLM: %v", err) + logme.DebugFln("Error getting answers from Gemini LLM for mandatory questions: %v", err) return nil, nil } - for _, answer := range answers { + for _, answer := range mandatoryAnswers { if answer.ShortAnswer != answer.ExpectedShortAnswer { - - var detailParts []string - - detailParts = append(detailParts, answer.Answer) - - if answer.CodeSnippet != "" { - detailParts = append( - detailParts, - fmt.Sprintf("**Code Snippet:**\n```\n%s\n```", answer.CodeSnippet), - ) - } - - if len(answer.Files) > 0 { - detailParts = append( - detailParts, - fmt.Sprintf("**Files:** %s", strings.Join(answer.Files, ", ")), - ) - } - - detail := strings.Join(detailParts, "\n\n") - + detail := buildDetailString(answer) pass.ReportResult( pass.AnalyzerName, llmIssueFound, @@ -199,5 +225,50 @@ func run(pass *analysis.Pass) (any, error) { } } + // Process optional questions (non-blocking warnings) + var optionalAnswers []llmvalidate.LLMAnswer + optionalAnswers, err = llmClient.AskLLMAboutCode(sourceCodeDir, OptionalQuestions, []string{"src", "pkg"}) + if err != nil { + logme.DebugFln("Error getting answers from Gemini LLM for optional questions: %v", err) + return nil, nil + } + + for _, answer := range optionalAnswers { + if answer.ShortAnswer != answer.ExpectedShortAnswer { + detail := buildDetailString(answer) + pass.ReportResult( + pass.AnalyzerName, + llmWarningFound, + fmt.Sprintf("LLM suggestion: %s", answer.Question), + detail, + ) + } + } + return nil, nil } + +// buildDetailString constructs the detail message for a reported issue +func buildDetailString(answer llmvalidate.LLMAnswer) string { + var detailParts []string + + detailParts = append(detailParts, answer.Answer) + + if answer.CodeSnippet != "" { + detailParts = append( + detailParts, + fmt.Sprintf("**Code Snippet:**\n```\n%s\n```", answer.CodeSnippet), + ) + } + + if len(answer.Files) > 0 { + detailParts = append( + detailParts, + fmt.Sprintf("**Files:** %s", strings.Join(answer.Files, ", ")), + ) + } + + detail := strings.Join(detailParts, "\n\n") + + return detail +}