Repository navigation
bug(ingest): unsafe/empty table-name rejections silently drop instead of routing to DLQ #151
Description
Problem
internal/ingest/bento.go:108:
// Validate table name to prevent SQL injection.
if !safeIdentifierRe.MatchString(raw.TableName) {
slog.WarnContext(msgCtx, "rejecting message with unsafe table name", "table", raw.TableName)
// TODO: manually push to a DLQ subject with metadata for later analysis instead of silently dropping?
if doubleAckErr := m.DoubleAck(msgCtx); doubleAckErr != nil {
slog.WarnContext(msgCtx, "double ack failed for dropped message", "error", doubleAckErr)
}
continue
}When the table name fails the safeIdentifierRe check, the message is logged at WARN and DoubleAck'd — i.e. permanently removed from JetStream — with no DLQ entry and no payload retained anywhere. Per AGENTS.md design decision #6 the DLQ exists precisely "to prevent silent data loss." This path violates that invariant.
The adjacent empty-table_name branch (bento.go:97-103) has the same shape and the same gap.
Proposed Solution
Route both rejection branches through the existing DLQ output (stream WAVEHOUSE_DLQ, subject dlq.<table> — falling back to dlq.__rejected__ when the table name itself is the reason for rejection). Include the rejection reason in headers (x-wh-drop-reason: unsafe-table-name / empty-table-name) so operators can grep the DLQ for these specifically.
Acceptance criteria
- No code path in
bento.gosilently drops a message — every drop either lands in the DLQ or surfaces with full payload atERROR. - Tests cover both the unsafe-table-name and empty-table-name DLQ paths.
- AGENTS.md design decision ci: bump goreleaser/goreleaser-action from 6 to 7 #6 wording matches the new behavior (or stays as-is if the new behavior matches the stated invariant).
Activity
coderabbitai commentedon May 18, 2026
🔗 Related PRs
#122 - fix(ingest): break infinite-Nak loop on permanent delete errors [merged]
#124 - chore(api)!: drop hub wildcard fan-out; SSE/WS use ?table= (closes
#100) [closed]📝 Issue Planner
Check the box below or use the
@coderabbitai plancommand to generate an implementation plan and prompts that you can use with your favorite coding assistant.🧪 Issue enrichment is currently in open beta.
You can configure auto-planning by selecting labels in the issue_enrichment configuration.
To disable automatic issue enrichment, add the following to your
.coderabbit.yaml:💬 Have feedback or questions? Drop into our discord!
EricAndrechek commentedon May 21, 2026
I think we prefer to handle table name validation at ingest, and assert that as part of the ingest api contract bento will ALWAYS have a table name – so if handled at the API layer they should return errors via the API and fail validation, not making it to the DLQ. Additionally, with the advent of #167, we really should only be rejecting table names that don't exist – and any table name could exist except for an empty string.
refactor(table names): handle unsafe table names (#174)