security(control-plane): error leakage, upload limits, WS cap, filename sanitization#96
Merged
Conversation
…me sanitization Addresses the control-plane hardening items (area 3, issue #8). - CP-3: DB errors no longer interpolate raw sqlx::Error into responses (schema disclosure). All DB call sites route through the existing generic `From<sqlx::Error>` mapper, which logs the detail server-side and returns a generic 500. Input-parse errors (YAML/spec) keep their user-facing detail. - CP-4: explicit request-body limits via DefaultBodyLimit — 1 MiB default for JSON endpoints, 32 MiB for spec/plugin upload routes — bounding in-memory buffering. - CP-6: cap concurrent WebSocket sessions (semaphore in ConnectionManager) so unauthenticated sockets can't pile up during the 30s registration window; sanitize upload filenames to a safe basename, defeating path traversal into the compile temp dir (worker.rs join) and CRLF/quote injection in the spec download Content-Disposition header. Sanitized at ingestion and at each use. CP-2 residual (per-project ownership / IDOR) is deferred: it needs the multi-tenant authz model and is not exploitable in the current single-admin deployment. Left tracked in #8.
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.
Area 3 — control plane (#8)
Continues the sequential security work, fail-closed by default.
CP-3 — DB error / schema leakage
Many handlers interpolated the raw
sqlx::Errorinto the response (internal_error_with_detail(format!("...{e}"))), disclosing schema/query internals. All DB call sites now route through the existing genericFrom<sqlx::Error>mapper (via?), which logs the detail server-side and returns a generic 500. Input-parse errors (YAML/spec) keep their user-facing detail, since those describe the caller's own input.CP-4 — unbounded upload buffering
Added explicit
DefaultBodyLimitlayers: 1 MiB default for JSON endpoints, 32 MiB for the spec/plugin upload routes (POST /specs,POST /projects/{id}/specs,POST /plugins). Bounds how much a caller can make the control plane buffer in memory per request.CP-6 — WS pre-auth pile-up + filename injection
ConnectionManagerbounds concurrent WebSocket handlers (registered and pending-registration). Unauthenticated sockets can no longer accumulate during the 30s registration window; excess connections are shed.safe_filenamereduces an untrusted upload filename to a safe basename — defeating (a) path traversal when the name isjoined onto the compile temp dir (worker.rs, a real arbitrary-file-write vector) and (b) CRLF/quote injection when reflected into the spec-downloadContent-Dispositionheader. Applied at ingestion (multipart) and defensively at each use. Unit-tested.Deferred
CP-2 residual (per-project ownership / IDOR): needs the multi-tenant authz model (project-scoped credentials + ownership checks); not exploitable in the current single-admin deployment. Kept tracked in #8.
Verification
cargo fmt --all --check,cargo clippy --workspace --lib --bins --exclude barbacane-test -D warnings, andsafe_filenameunit tests all green. The new Security Suite CI job (from PR #95) boots the control plane for the authz category, exercising these changes end-to-end.