security(E5): centralize SSRF guard + path traversal tests (T04+T05) - #22
Merged
Conversation
Split 55+ files exceeding the 300-line cap into co-located domain modules (same `package internal`, no import changes in callers). All 31 plugin builds green. Documented SQL-schema exceptions: stripe/db_schema.go (761L), github/db_schema.go (608L), torrent-manager/db.go (392L), content-acquisition/db.go (341L), vpn/db_schema.go (320L), donorbox/db.go (303L), paypal/db.go (301L) — all contain indivisible `const schemaSQL` / `Migrate` SQL blocks. Tickets: P4-E0-W1-S02-T02
…ry format
normalizeToArray() used Object.values() which dropped dict keys (plugin names)
when processing the pro registry format {\"pluginName\": {version, ...}}.
Changed to Object.entries() so the dict key becomes the fallback name when
the entry omits an explicit name field. This matches the existing TypeScript
source in registry.ts (which already used entries()).
This fix enables pro plugins (claw, claw-budget, etc.) to appear by name
in the combined free+pro registry served at plugins.nself.org.
Closes P4-E4-W3-S08-T01 [11] registry name resolution gap
…ical trio Add Plugin Rename Policy section to Deprecation-Policy.md with 5-alias retirement table and migration path. Update Plugin-Marketplace.md with canonical trio (nself-ai-cc/3760, nself-ai-gateway/3761, nself-ai-mcp/3762) replacing retired plugin-ai entry. Bundle membership updated.
…g check - Add runRustGates: cargo clippy --deny warnings + cargo test --all-features - Detect Rust stack via Cargo.toml presence in detectStacks - Add GatewayBase to Config; append gateway routing check after stack gates - Add run subcommand: discovers .ci.yaml manifests and runs stages in canonical E7 order (plugin-retrieval → plugin-gauth → plugin-clawde-pty) - Add --env staging|local flag to resolve gateway base URL (never production IP) - Add pipeline.go: DiscoverAndRunPipeline, zero-dep .ci.yaml parser - SPORT: PLUGINS-CI-004 (rust gates) PLUGINS-CI-005 (gateway check) PLUGINS-CI-006 (pipeline) - P4-E7-W4-S22-T13 CI gates go build + go vet pass
…ntID Single-spelling header check silently merged tenants whose gateway forwarded X-Source-Account-ID / X-Source-Account-Id / X-Source-Account instead of X-Hasura-Source-Account-Id. Match canonical e2ee auth.go 4-spelling logic to enforce multi-tenant isolation.
User-supplied webhook URLs were passed to http.DefaultClient with no validation, allowing requests to cloud metadata (169.254.169.254), RFC1918, and loopback addresses. Add validateWebhookURL deny-list check (mirrors free/webhooks dispatcher) before the request and a redirect-refusing client so a public host cannot bounce to an internal one. Dev opt-in via NOTIFY_ALLOW_PRIVATE_URLS=true.
config.sh and run.sh inherited the plugin's full os.Environ(), exposing HASURA_ADMIN_SECRET, DATABASE_URL, NSELF_LICENSE_PRIV_HEX and Stripe keys to untrusted GitHub Actions workflow code. Replace with runnerEnv(): an allowlist of safe base vars (PATH/HOME/LANG/...) plus GITHUB_RUNNER_*/ RUNNER_* prefixes, with a hard deny-list (HASURA/DATABASE/NSELF_LICENSE/ STRIPE/SECRET/TOKEN/KEY/PASSWORD/PAT/...) that wins over allows.
VideoPath/SubtitlePath/InputPath from request bodies were passed unsanitized to exec.CommandContext (alass/ffsubsync) and os file ops, allowing reads/writes of arbitrary host files via '..' or absolute paths. Add validateMediaPath containment guard (filepath.Clean + Rel, reject traversal/escape) and a SUBTITLE_MEDIA_ROOT config bound (default = storage path); apply at /v1/sync, /v1/qc, /v1/normalize, /v1/fetch-best.
Reject preview URLs resolving to private/internal addresses (RFC1918, loopback, link-local incl. 169.254.169.254 cloud metadata) before issuing the request, and re-validate every redirect hop. Dev opt-out via LINK_PREVIEW_ALLOW_PRIVATE_URLS=true. Pattern matches free/notify.
ValidateWebhookURL existed but was never called. Now enforced at endpoint create/update (pre-persist) and re-validated at delivery time (TestEndpoint + processPending) as DNS-rebinding second layer. Dev opt-out via WEBHOOK_ALLOW_PRIVATE_URLS=true. Added delivery-path block test.
…+ function size-cap exception docs
Two QA-C hard failures from P4-E0-W1-S02-T02 re-attempt:
CRITICAL: subtitle-manager, content-progress, torrent-manager, stripe all
used raw r.Header.Get("X-Source-Account-ID") (single spelling). All four
now delegate to sdk.SourceAccountID() / local 4-spelling loop. Stripe has
no sdk dep so uses a local sourceAccountHeaders slice mirroring the SDK
canonical pattern. No silent tenant merging when Hasura forwards a
different header casing.
HIGH: 135 functions >50L had zero per-function exception documentation.
Added Size-cap exception comments above each: SQL DDL migrations (linear
DDL — splitting breaks transaction semantics), main() entry points
(startup wiring, not reusable), HTTP route handlers (single-responsibility
decode+validate+db+encode), DB scan methods (single SQL query with struct
mapping), sync pipelines, config loaders, webhook routers.
All 29 plugins build green. No behavior changes.
…cal header spellings
…s, Hasura metadata, Dockerfiles T01 webhooks: CheckRedirect SSRF guard on http.Client (redirect-to-private blocked); source_account_id isolation column + 002 migration; Hasura tables.yaml with event trigger. T02 link-preview: source_account_id 002 migration; Hasura tables.yaml. SSRF already existed (ssrf.go + validatePreviewURL wired in extractor). T03 torrent-manager: ValidateAdminURL (ssrf.go) blocking private IPs + port allowlist; 001_initial.sql for all 9 np_torrentmanager_* tables; Hasura tables.yaml + event trigger. T04 ci: Dockerfile (golang:1.22-alpine builder + alpine:3.19 + gitleaks); 3 tests (stack detection + gitleaks-skip); Hasura stub (no DB tables). T05 monitoring: new Go module; cmd/ init-container renderConfigs; 2 tests; Dockerfile; Hasura stub. T06 ollama: cmd/ lifecycle handler (install idempotent via ollama list check); 2 tests; Dockerfile with docker-cli; README; source_account_id migration; Hasura tables.yaml. T07 family-gedcom: GEDCOM 5.5.1 parser (INDI+FAM); testdata/sample.ged fixture; 4 tests; Dockerfile; Hasura stub (stateless converter). All 7 tickets: go build ✅ all modules, 55 tests pass across 7 packages. SPORT REGISTRY-PLUGINS.md updated for all 7 plugins.
…solation Lock down the runnerEnv() deny-list with three unit tests: - ForbiddenKeysAbsent: HASURA_ADMIN_SECRET, DATABASE_URL, NSELF_LICENSE_PRIV_HEX must never reach the runner subprocess environment. - SafeKeysPresent: RUNNER_WORKSPACE and GITHUB_WORKFLOW pass through. - DenyBeatsAllow: RUNNER_SECRET is blocked by deny-substring even when the allow-prefix RUNNER_ would otherwise permit it.
Adds path_guard_test.go with 5 test functions covering ../../../etc/passwd, absolute paths outside root, symlinks, empty input, and valid paths. Validates the existing validateMediaPath(root, p) fix is covered by tests.
… notify+webhooks (T-P4-E5-04) Creates plugins/sdk/httpx/ssrf.go as the canonical ValidateOutboundURL implementation (RFC1918, CGNAT, link-local, IMDS, loopback, IPv6 reserved). Rewrites notify/internal/ssrf.go and webhooks/internal/dispatcher.go to delegate to sdk/httpx instead of maintaining independent implementations. Canonicalizes env bypass var to NSELF_ALLOW_PRIVATE_URLS across all tests.
…DQA)
Fixes T-P4-E5-W1-S01-T06 test failure: runnerEnv() was dropping PATH because
the deny-substring check ("PAT") ran before the exact-match allow check.
PATH contains "PAT" as a substring, so it was silently denied despite being
in runnerEnvAllowExact. Fix: check exact-match first, then deny, then prefix.
All 12 internal tests now pass including TestRunnerEnv_SafeKeysPresent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sdk/httpx/ssrf.goas canonicalValidateOutboundURL(RFC1918, CGNAT, IMDS, loopback, IPv6 reserved). Migratesnotifyandwebhooksplugins to delegate to it. Removes ~200 lines of duplicate SSRF logic.path_guard_test.gofor subtitle-manager with 5 test functions covering../../../etc/passwd, absolute paths, symlinks, empty input, and valid paths.NSELF_ALLOW_PRIVATE_URLSacross all tests.QA
go test ./httpx/...(sdk): 17 passedgo test ./internal/...(notify): 9 passedgo test ./internal/...(webhooks): 14 passedgo test ./internal/...(subtitle-manager): 16 passed