Skip to content

Unauthenticated POST /graphql leaks raw sqlx/Postgres error text in query resolver errors (sibling of #226/#106) #250

Description

@beardthelion

An unauthenticated POST /graphql returns raw sqlx/Postgres error text in the response body when any query resolver's database call fails. Same class as #226 and #106, on a surface neither of them covers, and one that does not route through AppError at all.

Mechanism

Every resolver on QueryRoot maps database failures straight into the GraphQL error message:

// crates/gitlawb-node/src/graphql/query.rs:17
let repos = db
    .list_all_repos_deduped()
    .await
    .map_err(|e| async_graphql::Error::new(e.to_string()))?;

The db layer propagates sqlx through anyhow with a bare ? and no .context(), so e.to_string() is the sqlx Error Display verbatim. Six sites in query.rs do this: lines 17, 30, 74, 89, 119, 128.

Because this path builds an async_graphql::Error directly, it bypasses AppError and its IntoResponse entirely. Opaque-ifying the AppError arms does not touch it.

Reachability

/graphql is mounted with optional_signature only:

// crates/gitlawb-node/src/server.rs:61-66
let graphql_routes = Router::new()
    .route("/graphql", get(graphql_playground).post(graphql_handler))
    .layer(middleware::from_fn(auth::optional_signature))

and optional_signature (crates/gitlawb-node/src/auth/mod.rs:251-258) only enforces when signature headers are present, passing a header-less request straight through to next.run(request). So an anonymous POST /graphql reaches every query resolver.

Reproduction

Executed against a real Postgres on main (111cff7), driving the schema with no AuthenticatedDid in context, which is how this repo's existing anonymous-GraphQL guards model an unauthenticated caller (graphql_repos_hides_private_from_anonymous, #97):

sqlx::query("ALTER TABLE repos DROP COLUMN is_public").execute(&pool).await.unwrap();
let resp = schema.execute("{ repos { name ownerDid } }").await;

Observed response:

{"data":null,"errors":[{"message":"error returned from database: column \"is_public\" does not exist","locations":[{"line":1,"column":3}],"path":["repos"]}]}

Any query- or connection-level error on those calls returns Postgres schema, constraint, or connection detail to an anonymous caller.

Scope

query.rs is the anonymous surface and the reason this is filed. mutation.rs has the same six-site pattern, but each mutation calls require_signer before touching the database, so it needs a valid signature first. Identities here are permissionless, so that is a lower tier rather than a real barrier, and it is worth fixing in the same pass. subscription.rs has no such sites.

Relationship to the siblings

#226 names get_by_cid's two AppError::Internal arms and asks for opaque AppError::Internal bodies. #247 does that and also opaque-ifies the Db arm, which is the right fix for what #226 describes. Neither reaches this surface, so #226 closing does not close this. Worth noting that #226's own notes predict that opaque-ifying AppError::Internal resolves the class; that is not true for GraphQL, which never routes through AppError.

#106 is the same shape on the smart-HTTP git endpoints via AppError::Git, also still open.

Fix

Stop passing the raw error into the client-visible message. Log the real error server-side and return a generic string, mirroring what #247 lands for AppError:

.map_err(|e| {
    tracing::error!(error = %format!("{e:#}"), "graphql query failed");
    async_graphql::Error::new("a database error occurred")
})?

A shared helper is preferable to twelve call sites repeating it. Use {e:#} rather than %e in the log so the anyhow cause chain survives, the way api/repos.rs:217 already does.

Severity

Calibrated against the siblings rather than asserted: #226 (raw DB error text, unauthenticated) is sev:high, #106 (filesystem path) is sev:low. This one leaks DB error text to an unauthenticated caller, so it matches #226.

Metadata

Metadata

Assignees

No one assigned

    Labels

    crate:nodegitlawb-node — the serving node and REST APIkind:securityVulnerability fix or hardeningsev:highMajor break or real security/trust risk, no easy workaroundsubsystem:apiNode REST API request/response surface

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions