[Self-Improve] signalpilot/improvements-round-2026-04-03-f245ef#55
Closed
[Self-Improve] signalpilot/improvements-round-2026-04-03-f245ef#55
Conversation
…aders, CORS fixes - Add RequestIDMiddleware for log correlation via X-Request-ID header - Add AuthBruteForceMiddleware: blocks IPs after 10 failed auth attempts in 5min - Add global exception handler to prevent stack trace leaks in responses - Add missing security headers: CSP, HSTS, Permissions-Policy - Fix CORS: add Vary:Origin for correct cache behavior - Improve rate limiter: O(1) deque pops, X-RateLimit-Limit/Remaining headers - Protect settings endpoint: prevent disabling auth by clearing API key - Preserve redacted "***" placeholders when frontend sends settings back - Fix public path matching to handle trailing slashes and subpaths - Log failed auth attempts with client IP for security monitoring - Update middleware tests (20 passing) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…x error handling - Add ConnectionName type (Annotated[str, Path(...)]) for validated path parameters - Apply ConnectionName to 40 endpoints across connections, schema, budget, cache routes - Replace raw dict with ImportManifest Pydantic model on import endpoint (max 100 connections) - Truncate error messages to 200 chars in import to prevent info leaks - Add audit trail for credential exports (compliance) - Wrap sandbox create/execute/kill in try/except to prevent infrastructure detail leaks - Sandbox kill errors are logged but don't block deletion cleanup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dation, path traversal Fixes critical issues found during code review: 1. Client IP extraction: centralized into module-level _client_ip(), ignores X-Forwarded-For by default (SP_TRUSTED_PROXY_COUNT=0). When behind a trusted proxy, uses Nth-from-right entry instead of blindly trusting the first entry. 2. Brute-force memory growth: added periodic _cleanup_stale() that removes expired blocks and stale failure entries. Caps tracked IPs at 10,000 to prevent memory exhaustion. 3. API key validation: strip whitespace before saving, enforce minimum 16-char length for new keys, prevents auth bypass via whitespace-only keys. 4. Public path matching: switched from prefix matching to exact match against explicit allowlist. Prevents path traversal bypass via /docs/../../api/query. Tests updated: 25 passing (5 new tests for cleanup and IP extraction). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…h strength Security fixes across the gateway module: 1. SSH tunnel command injection (connectors/ssh_tunnel.py): - Validate proxy_host and ssh_host against strict hostname regex - Use shlex.quote() for defense-in-depth on ProxyCommand args - Reject hostnames with shell metacharacters before building command 2. Metrics URL leak (api/metrics.py): - Replace sandbox_manager_url with sandbox_manager_configured boolean - Prevents internal infrastructure URLs from leaking via SSE stream - Add ConnectionName validation to capabilities endpoint 3. DATA_DIR permissions (store.py): - Create data directory with mode 0o700 (owner-only) - Tighten permissions on existing directories 4. PII hash brute-force (governance/pii.py): - Use full 64-char SHA-256 hex digest instead of truncated 12-char - Prevents offline brute-force of low-entropy values (phones, SSNs) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The credential export audit trail was using request.client.host directly, which records the proxy IP instead of the real client IP when running behind a reverse proxy with SP_TRUSTED_PROXY_COUNT > 0. Now uses the centralized _client_ip() function for consistent IP extraction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Block tunneling of privileged ports (1-1023) to prevent exposing SSH, database, and other internal services via Cloudflare tunnels - Cap audit offset parameter to 50,000 for consistent validation - Add ge=0 constraint to audit limit/offset parameters Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comprehensive test coverage for gateway security features: - SSH hostname validation (12 tests): valid forms, injection payloads - Proxy command building (8 tests): shlex quoting, malicious input rejection - PII hash output (10 tests): full SHA-256 digest, determinism - Connection name validation (15 tests): valid/invalid names, path traversal - Tunnel port blocking (9 tests): privileged port enforcement - Settings API key logic (16 tests): stripping, placeholder, min length All 70 tests pass alongside existing 25 middleware tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The PII hash was changed from truncated 12-char to full 64-char digest. Update existing test assertion to match the new output format. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The /health endpoint is public (unauthenticated) and was leaking: - sandbox_manager_url (internal infrastructure URL) - full error messages from sandbox health checks - counts of connections and sandboxes Now returns only status, version, and sandbox health status. Error messages are reduced to just "error" to prevent info leaks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Limit error message length to 200 chars when ConnectionCreate validation or connection string building fails, preventing potential information leakage of internal model details. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…validation Addresses remaining findings from security audit: 1. Brute-force protection now tracks ALL methods (not just POST/PUT/DELETE) since auth brute-forcing is method-agnostic 2. Clone connection new_name parameter now validated with same regex pattern as ConnectionName (alphanumeric, hyphens, underscores) 3. Tunnel port allowlist enforced (was dead code). Only explicitly allowed ports (3000, 3200, 3400, 3401, 8180) can be tunneled. Prevents accidental exposure of databases or admin interfaces 4. Added ResourceID type for UUID path parameter validation on tunnel_id (4 endpoints) and sandbox_id (3 endpoints) 5. Updated security tests for allowlist-based port validation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…pection tests - Append Vary: Origin instead of overwriting (preserves existing Vary headers) - Sanitize global exception handler logs via sanitize_db_error to strip connection strings and credentials before logging - Replace inspect.getsource() tests with TestClient integration tests that verify actual response headers and middleware behavior (27 new tests) - Fix health endpoint test to match stripped-down public response Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
9 new integration tests for APIKeyAuthMiddleware using TestClient: - Dev mode (no key) passes with X-SignalPilot-Auth: none - Valid Bearer token and X-API-Key headers accepted - Missing key returns 401 with helpful detail message - Wrong key returns 403 with detail message - Public paths and OPTIONS bypass auth Total middleware tests: 58 (up from 49). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When _failures dict reaches _MAX_TRACKED_IPS capacity after cleanup, skip recording new IPs entirely. This prevents distributed brute-force attacks from exhausting server memory by sending failures from millions of unique IPs concurrently (where no entries are stale enough to clean). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add _validate_hostname() call for remote_host in SSH tunnel setup for defense-in-depth consistency with ssh_host/proxy_host validation - Remove X-SignalPilot-Auth: none response header that advertised auth-disabled state to attackers; log warning server-side instead - Update test to verify auth state is NOT leaked in response headers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove outer [:500] slice before sanitize_db_error — truncating before sanitization can split credential patterns mid-token, evading regex - Check existing Vary header before appending to prevent duplicate entries - Remove dead _client() method from auth middleware tests - Add explanatory comment on patch target for late-import mock Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
2 participants
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.
Self-Improvement Run
Branch:
signalpilot/improvements-round-2026-04-03-f245efRun ID:
40df1a7c-9369-44cb-b6a0-03cadcb0c5efThis PR was created by the self-improvement agent.
Review all changes carefully before merging to
main.Generated by Self-Improve Framework