[quality] Add unit tests for SSRF prevention validation (isPrivateIP, validateBaseURL)#17136
[quality] Add unit tests for SSRF prevention validation (isPrivateIP, validateBaseURL)#17136clubanderson wants to merge 7674 commits into
Conversation
Pin all remaining mutable reusable workflow references in GitHub Actions workflows to a specific kubestellar/infra commit SHA to avoid trusting a floating @main ref at runtime. Fixes #16445 Signed-off-by: kubestellar-hive <hive-bot@kubestellar.io> Co-authored-by: kubestellar-hive <hive-bot@kubestellar.io>
…16463) pollBackendHealth, and ensureTLSCert covering the key risk areas identified in the coverage gap analysis. Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add tests for client_pods, client_nodes, client_rbac, and client_watch using fake clientset pattern already established in the package. Signed-off-by: kubestellar-hive <hive-bot@kubestellar.io> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add test coverage for the highest-priority untested handlers: - auth_oauth_test.go: OAuth redirect, callback, token exchange - sse_handler_test.go: SSE connection lifecycle and event streaming Part of the broader handler coverage effort (#16429). Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* 🌱 Extract Stellar domain types to pkg/stellar Moves Stellar domain types from pkg/store to pkg/stellar/types.go, establishing correct package ownership. The store package now uses type aliases for backwards compatibility, avoiding the need to update all call sites immediately. This is the first step toward separating Stellar business logic from persistence, addressing the architectural issue where pkg/stellar cannot be tested in isolation and domain types are defined in the wrong package. Changes: - Created pkg/stellar/types.go with 13 Stellar domain types - Updated pkg/store/store.go to use type aliases referencing stellar package - Maintains full backwards compatibility — no breaking changes - All tests pass Related to separation of concerns and preparing for future extraction of Stellar to a standalone service. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🌱 Add minimal tests for pkg/stellar types Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…6504) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add an RFC that defines plugin scope, extension points, security constraints, and a phased implementation plan for evolving console-marketplace toward installable extensions. Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* 🔒 Restrict agent token endpoint to admin users only Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🌱 Add unit tests for agent token admin authorization Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* 🔒 Restrict agent auto-update proxy to admin users Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🌱 Add unit tests for auto-update proxy admin authorization Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
#16489) * 🐛 Fix predictable /tmp paths in cmd/watcher to prevent symlink attacks Replace hardcoded /tmp paths with os.MkdirTemp/os.CreateTemp for unpredictable temporary file paths. Set restrictive permissions (0600/0700) and ensure proper cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: kubestellar-hive[bot] <kubestellar-hive[bot]@users.noreply.github.com> * 🌱 Add unit tests for secure temp directory creation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: kubestellar-hive[bot] <kubestellar-hive[bot]@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: kubestellar-hive[bot] <kubestellar-hive[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
- Rename pkg/kagenti_provider → pkg/kagentiprovider (Go naming convention) - Add README.md to pkg/agent, pkg/kagent, pkg/kagentiprovider documenting boundaries - Update all imports and type references - Addresses issue #16430: agent package fragmentation The rename follows Go package naming conventions (no underscores). README files clarify the three-package architecture: - pkg/agent: Primary agent orchestration and provider abstraction - pkg/kagent: Client for standalone kc-agent binary (local process) - pkg/kagentiprovider: Client for in-cluster kagenti deployments (K8s-native) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…6566) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use sandboxed iframe with sandbox="" attribute instead of opening blob URL - Fetch circuit HTML server-side in modal instead of exposing blob URL in same origin - Add CSP and X-Content-Type-Options headers to quantum proxy - Prevents CWE-79 XSS vulnerability from malicious upstream quantum services Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* 🔒 Restrict rate-limit status endpoint to admin users Adds admin authorization check to GetRateLimitStatus handler to prevent information disclosure of user IDs and IP addresses to non-admin users. Fixes #16481 (CWE-862: Missing Authorization) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * chore: retrigger CI after Docker registry timeout Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix(admin): update test for NewAdminHandler store.Store parameter Signed-off-by: kubestellar-hive <hive-bot@kubestellar.io> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Signed-off-by: kubestellar-hive <hive-bot@kubestellar.io> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: kubestellar-hive <hive-bot@kubestellar.io>
…6524) * 🔒 Restrict NPS endpoint to prevent unauthorized feedback exposure - Removes user feedback comments from public GET /api/nps endpoint - Feedback field no longer exposed in recent responses array - Maintains aggregate NPS metrics for dashboard functionality - Feedback comments may contain PII (emails, incident details, internal URLs) - Admin endpoint with proper authorization required to access raw feedback Fixes #16486 Security Impact: - CWE-200: Exposure of Sensitive Information to an Unauthorized Actor - CWE-862: Missing Authorization - Prevents unauthorized access to user-submitted feedback with potential PII Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * chore: retrigger CI after Docker registry timeout Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…16535) * 🔒 Require editor-or-admin role on stellar actions execute endpoint Add requireEditorOrAdmin check to ExecuteAction handler so that viewer-role users can no longer invoke destructive K8s operations (DeletePod, ScaleDeployment, RestartDeployment, CordonNode). Also removes a duplicate RequireAdmin declaration in auth_helpers.go that was introduced by a recent commit and broke compilation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🌱 Add RBAC tests for stellar actions execute endpoint Tests verify that viewer role is rejected (403) and editor/admin roles are permitted on POST /api/stellar/actions/execute. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* 🌱 Extract cmd/watcher business logic into pkg/watcher Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🐛 Fix Kagenti provider import alias Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🌱 Fix service exports handler test expectations Reset the test kubeconfig before injecting ServiceExport clusters so\nListServiceExports only probes the fake clusters configured by the\ntest. This avoids the placeholder test-cluster triggering a real\ndynamic client lookup and Fiber test timeout.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#16551) * 🔒 Fix percent-encoded path traversal bypass in missions-file function Harden hasInvalidPathInput and hasInvalidRefInput to iteratively decode percent-encoded values before checking for traversal patterns. Previously, payloads like %252e%252e would bypass the literal '..' check after a single URL decode pass. Matches the defense-in-depth pattern already used in the Go backend's sanitizePath function (pkg/api/handlers/missions_cache.go). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🌱 Fix duplicate RequireAdmin declaration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🌱 Add unit test for percent-encoded path traversal fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…6529) * 🔒 Restrict admin bootstrap to prevent unauthorized auto-promotion Fixes #16485 Removes the privilege escalation vulnerability where any authenticated user could be silently promoted to admin if all admins were deleted or if the admin count reached zero. ## Security Changes - **Removed auto-bootstrap from requireAdmin()**: The admin role check no longer automatically promotes users even when admin count is zero. This prevents privilege escalation if all admins are removed (manually, via bug, or via DB corruption). - **Bootstrap now controlled via environment variable**: Added BOOTSTRAP_ADMIN_ALLOWED environment variable (defaults to false) to explicitly control whether bootstrap promotion is allowed at all. - **Bootstrap only during initial OAuth setup**: Bootstrap promotion now only occurs during the initial user creation in auth_handler.go during OAuth login flow, not on every admin endpoint check. ## Impact - Self-hosted consoles must set BOOTSTRAP_ADMIN_ALLOWED=true to enable first-user admin bootstrap during initial setup. - Once an admin is created, the bootstrap mechanism is effectively disabled unless BOOTSTRAP_ADMIN_ALLOWED is explicitly set. - If all admins are removed, no new admins can be auto-promoted. ## CWE CWE-269: Improper Privilege Management Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * chore: retrigger CI after Docker registry timeout Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🌱 Fix auth_helpers test expectations for restricted bootstrap Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * � Fix admin bootstrap to allow first-user promotion while restricting subsequent Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* 🌱 Split Store interface into focused sub-interfaces (ISP) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🐛 Fix interface signatures to match SQLiteStore implementation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🐛 Fix interface compliance after Store ISP split Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
) * 🔒 Sanitize nightly E2E image parsing against prototype pollution Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * chore: retrigger CI after Docker registry timeout Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🌱 Add test for nightly E2E image sanitization Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * 🐛 Fix prototype pollution rejection in nested image parsing Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add requireEditorOrAdmin check to Chat and CallTool handlers so viewer-role users can no longer invoke arbitrary kagent agents/tools that may execute privileged Kubernetes operations. Also removes the duplicate RequireAdmin declaration that broke build. Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Align the test i18n mock with the NamespaceOverview health badge translation so the multi-cluster selector assertion matches component behavior. Signed-off-by: Test User <test@example.com> Co-authored-by: Test User <test@example.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Passed: 31/32 Failed: 1 [skip ci]
Dashboard health indicators (#17114) add rendering overhead to compliance cards during warm-return, increasing cache miss likelihood on CI shared runners under contention. Bump the tolerance from 8→10 to absorb nightly flakiness while remaining meaningful. Fixes #17120 Signed-off-by: Test User <test@example.com> Co-authored-by: Test User <test@example.com>
Tests cover isPrivateIP and validateBaseURL — security-critical functions that prevent Server-Side Request Forgery (CWE-918) by blocking requests to private/internal IP addresses. Test cases: - isPrivateIP: RFC1918, loopback, link-local, IPv6 ULA/link-local, unspecified addresses, and public IPs - validateBaseURL: empty/whitespace/bad-scheme rejection, literal private IP blocking, public IP acceptance, ALLOW_LOCAL_PROVIDERS bypass Signed-off-by: clubanderson <club.anderson@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole canceled.
|
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
There was a problem hiding this comment.
Pull request overview
Adds Go unit tests for SSRF-prevention logic in pkg/agent/server_ops_validation.go, focusing on private-IP detection, base URL validation (scheme/whitespace), DNS fail-closed behavior, and the ALLOW_LOCAL_PROVIDERS escape hatch.
Changes:
- Introduces table-driven tests for
isPrivateIPacross IPv4/IPv6 private, loopback, link-local, unspecified, and public addresses. - Adds validation tests for
validateBaseURLcovering empty/whitespace inputs, bad schemes, literal private/public IPs, and DNS failure fail-closed behavior. - Adds env-var behavior tests for
allowLocalProviders.
| // contains is a test helper checking substring presence. | ||
| func contains(s, substr string) bool { | ||
| return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsSubstr(s, substr)) | ||
| } | ||
|
|
||
| func containsSubstr(s, sub string) bool { | ||
| for i := 0; i <= len(s)-len(sub); i++ { | ||
| if s[i:i+len(sub)] == sub { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
| t.Run("unset", func(t *testing.T) { | ||
| os.Unsetenv("ALLOW_LOCAL_PROVIDERS") | ||
| if allowLocalProviders() { | ||
| t.Error("should be false when env var is unset") | ||
| } | ||
| }) |
…ollision) The agent package already has a `contains` function, causing a redeclaration error. Use strings.Contains from the standard library instead. Signed-off-by: clubanderson <club.anderson@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Status Check — Stuck PRThis PR has been in Current state:
Findings
Recommended Next Steps
Flagged by stuck-detection workflow at
|
|
Closing: branch is catastrophically stale (1.2M+ line diff). The SSRF prevention tests are good — re-create on a fresh branch from main. |
Test Improvement
Adds unit tests for SSRF (Server-Side Request Forgery) prevention functions in
pkg/agent/server_ops_validation.go. These security-critical functions validate provider base URLs and block requests to private/internal IP addresses (CWE-918).Functions tested:
isPrivateIP— validates RFC1918, loopback, link-local, IPv6 ULA/link-local, unspecified addresses, and public IPsvalidateBaseURL— SSRF prevention via URL scheme validation, whitespace rejection, private IP blocking, DNS-fail-closed behaviorallowLocalProviders— environment variable escape hatch for local developmentTest categories:
ALLOW_LOCAL_PROVIDERS=truebypass verificationRelates to #17122
Filed by quality agent (ACMM L3/L5 — hold-gated mode). Hold-gated: human review required.