fix(security): 0.1.0 hardening pass (9 findings)#95
Open
dviejokfs wants to merge 1 commit into
Open
Conversation
Fixes 4 CRITICAL, 5 HIGH, 1 MEDIUM security issues identified in the 0.1.0 release audit. All fixes verified with cargo check and unit tests. CRITICAL - agents: bound agent run token expiry to (timeout + 120s) and revoke on every run exit (success/failure/cancel/no-changes). Token still carries FullAccess permission because the workspace memory endpoints reject non-FullAccess deployment tokens at the auth layer; TODO left for a purpose-built AgentRunWrite permission in 0.2.0. - ai-gateway: validate BYOK X-Provider-Base-URL through temps_core::url_validation::validate_external_url before handing to reqwest. Blocks cloud metadata (169.254.169.254), loopback, RFC-1918, link-local, multicast, non-http(s) schemes. - deployments: replace pg_notify format!() + manual apostrophe-escape with Statement::from_sql_and_values bound parameters in mark_deployment_complete; document the two remaining call sites (hardcoded table identifiers, constant NOTIFY channel) as safe. HIGH - git: delete unsigned legacy /webhook/github route and handler. The verified github_webhook_events endpoint remains authoritative. - git: reject GitLab webhooks for projects with no stored signing token (was: None => true). Operators must re-enroll legacy projects. - auth: extract resolve_client_ip into shared module; trust X-Forwarded-For only when the direct TCP peer is loopback. Mirrors the existing rate-limiter logic so audit logs cannot be spoofed by direct-internet clients. - deployments: validate worker-supplied private_address on node registration against reserved ranges (loopback, link-local incl. AWS metadata IP, multicast, broadcast, unspecified, IPv6 equivalents). RFC-1918 and public IPs still allowed. - git: replace Host-header reflection in GitHub OAuth/install callback redirects with state.config_service.get_external_url(). Removes open-redirect vector that allowed phishing post-install. MEDIUM - ai-gateway: scrub URLs from reqwest::Error via .without_url() in From<reqwest::Error> impl so BYOK keys/base_urls cannot leak into structured logs via http client failures. Tests added (40+ new): - temps-agents: revoke_run_token, run_token_ids map lifecycle - temps-deployments: revoke_token_by_id, node address validator (12 IP cases) + register_node handler integration - temps-ai-gateway: BYOK SSRF rejection (6 ranges) + accept (3 providers) - temps-auth: resolve_client_ip (7 cases) + 174 existing tests still pass - temps-git: project_has_signing_token (3 cases) Also fixes pre-existing clippy lint (gitlab.rs:265 explicit_auto_deref) that the pre-commit hook surfaced. Verified: cargo check --lib clean across all 42 workspace crates.
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
Security audit blocker pass for the 0.1.0 release. Addresses 4 CRITICAL + 5 HIGH + 1 MEDIUM findings from a focused security review of the OSS Rust backend.
The audit was scoped to OSS code paths and excluded
temps-ee/andtemps-landing/(a follow-up deeper sweep of SSRF surfaces, EE crates, and the Stripe webhook handler is recommended). Multi-tenancy was intentionally not treated as a finding — OSS is single-tenant by design (noowner_user_idonprojects); per-team isolation is the temps-ee teams Premium feature.Findings fixed
CRITICAL
temps-agents) — token expiry bounded totimeout + 120s(was hardcoded 2h) and revoked on every run exit path. FullAccess permission retained because workspace memory endpoints currently reject non-FullAccess deployment tokens at the auth layer;TODO(0.2.0)left for a purpose-builtAgentRunWritepermission.temps-ai-gateway) —X-Provider-Base-URLnow validated throughtemps_core::url_validation::validate_external_url. Blocks 169.254.169.254 (cloud metadata), loopback, RFC-1918, link-local, multicast, non-http(s) schemes.temps-deployments) — replacedformat!()+ manual apostrophe escape with boundStatement::from_sql_and_values. OtherStatement::from_stringsites in the crate audited and documented as safe (hardcoded table identifiers / constant NOTIFY channels).HIGH
/webhook/githubroute — deleted entirely.None => trueflipped toNone => false. Operators must re-enroll legacy projects.resolve_client_iphelper shared between rate-limit and auth middleware. Trusts X-Forwarded-For only when direct TCP peer is loopback.private_addressSSRF — node registration now validates against reserved IP ranges (loopback, link-local incl. AWS metadata IP, multicast, broadcast, unspecified, IPv6 equivalents).state.config_service.get_external_url().MEDIUM
reqwest::Error—From<reqwest::Error>impl now calls.without_url()to scrub URLs (and any embedded credentials) before stringifying.Out of scope for this PR
Audit recommendations for a follow-up deeper sweep:
temps-ee-auth-policy,temps-ee-license-api,temps-ee-teamsRBAC enforcement)temps-landing/Test plan
cargo check --libclean across all 42 workspace cratescargo test --lib -p temps-agents(new revoke + token tests pass)cargo test --lib -p temps-ai-gateway(145 tests pass, 6 new BYOK SSRF tests)cargo test --lib -p temps-auth(174 tests pass, 7 new client_ip tests)cargo test --lib -p temps-deployments(node validator: 12 IP cases + 2 handler integration tests)cargo test --lib -p temps-git(project_has_signing_token tests pass)external_urlfrom settings (set the env var first)X-Provider-Base-URL: http://169.254.169.254→ expect 400private_address: 169.254.169.254→ expect 400Migration / operator notes
external_urlin settings. The OAuth/install callback no longer falls back to Host header or localhost —external_urlmust be configured before the GitHub App install flow will complete.