feat(ingest): add hotdata ingest command group#206
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| let body = serde_json::to_value(req).expect("IngestRequest serializes"); | ||
| self.send( | ||
| self.authed(reqwest::Method::POST, "/sources").json(&body), | ||
| Some(&body), |
There was a problem hiding this comment.
Blocking (security): --debug leaks source credentials in plaintext.
create_source passes the full serialized IngestRequest as the body_log argument to send. util::send_debug → debug_request prints that body verbatim (see util.rs:60-65); it only masks the Authorization header and only redacts response fields via TOKEN_REDACT_KEYS. It never redacts request-body fields.
So hotdata ingest sql --url postgresql://user:PASSWORD@host/db --debug (or --config with a password, rest_config with a bearer token, catalog_config with a catalog token, or filesystem aws_secret_access_key) prints the secret in cleartext to stderr — exactly the output users paste into bug reports and CI logs.
This contradicts the repo's established convention: jwt.rs builds a redacted_form_body(¶ms) and passes that as the log body precisely because body_for_log is printed unredacted (its doc: "pass ... a hand-rolled redacted Value for form bodies"). The design doc §3 also promised token redaction here.
Fix: build a redacted clone of the body for logging (the redact_json_fields helper already exists), e.g. redact credentials/rest_config/catalog_config before passing to send, rather than Some(&body).
There was a problem hiding this comment.
Review
Blocking Issues
src/client/ingest.rs:204 — source credentials leak in plaintext under --debug.
create_source passes the full unredacted IngestRequest as the debug log body. util::send_debug/debug_request print request bodies verbatim (they only mask the Authorization header and only redact response fields via TOKEN_REDACT_KEYS). This exposes SQL passwords/connection strings, REST bearer tokens, Iceberg catalog tokens, and filesystem secret keys to stderr — the output users routinely paste into issues and CI logs. The rest of the CLI already avoids this (jwt.rs passes a redacted_form_body), and the design doc §3 promised redaction here.
Action Required
- Redact secret-bearing fields (
credentials,rest_config,catalog_config) from the request body before passing it tosendfor debug logging — reuse the existingredact_json_fieldshelper. Apply the same treatment to any other enqueue body that can carry secrets.
Drives the dlthubworker ingest API (/v1/ingest/*) through the gateway via a raw-HTTP client (routes aren't in the SDK yet): sql/rest/filesystem/ iceberg onboarding, restricted SQL queries, translate, connector catalog, status, drain, schema, preview, and parquet download. Reconciled against the stable post-gateway API (2026-07-07 rollout): - enqueue (POST /sources|/queries) sends the durable hd_ API key as the bearer — the server rejects JWTs there because the drain job outlives them; without an API key the CLI fails fast with a --api-key hint - verified live: the CLI's user-scoped session JWT only reaches /connectors — workspace-scoped endpoints 403 (the worker derives the workspace from the JWT's workspace:* scope and ignores X-Workspace-Id on the JWT route), so those 403s get an --api-key hint too - no `ingest sources` command: GET /ingest/sources never shipped - 2.5s status polling, drain re-kick after 30s pending (control-store read lag), brief 404 retry on `ingest query` right after onboarding, and a database-scoped-token hint when a load fails with Forbidden
eddietejeda
force-pushed
the
feat/ingest-commands
branch
from
1ee3697 to
8419569
Compare
| let body = serde_json::to_value(req).expect("IngestRequest serializes"); | ||
| self.send( | ||
| self.authed(reqwest::Method::POST, "/sources").json(&body), | ||
| Some(&body), |
There was a problem hiding this comment.
Blocking (security): still leaks source credentials in plaintext under --debug.
This is unchanged since the prior review. create_source passes Some(&body) — the full serialized IngestRequest — to send, and util::send_debug → log_request_struct → debug_request (util.rs:60-65) prints the request body verbatim. Only the Authorization header is masked; request-body fields are never redacted. redact_json_fields is still private in util.rs and is not used here.
So hotdata ingest sql --url postgresql://user:PASSWORD@host/db --debug (and rest_config bearer tokens, catalog_config tokens, filesystem aws_secret_access_key) still prints the secret to stderr — the exact output users paste into bug reports and CI logs.
Fix: build a redacted clone of the body for logging (mask credentials/rest_config/catalog_config) and pass that instead of Some(&body). This mirrors jwt.rs, which builds redacted_form_body(¶ms) precisely because the log body is printed unredacted. Making redact_json_fields (or a small wrapper) pub(crate) would let you reuse the existing helper.
There was a problem hiding this comment.
Review
Blocking Issues
src/client/ingest.rs:204—create_sourcestill passes the full unredactedIngestRequestbody (Some(&body)) to the debug logger, leaking source credentials (credentials,rest_configbearer tokens,catalog_configtokens, filesystemaws_secret_access_key) in plaintext to stderr under--debug. This was flagged in the prior review and remains unfixed.
Action Required
Build a redacted clone of the request body for logging (mask credentials/rest_config/catalog_config) and pass that to send instead of Some(&body), mirroring the redacted_form_body pattern already used in jwt.rs. The existing redact_json_fields helper in util.rs can be reused if exposed as pub(crate).
The enqueue body's credentials / rest_config / catalog_config subtrees (SQL passwords, connection strings, REST bearer tokens, catalog tokens, object-store keys) were printed verbatim to stderr under --debug. Log a view with those subtrees replaced by "***" instead; the wire body is unchanged. Mirrors jwt.rs's redacted_form_body pattern.
|
Fixed in |
There was a problem hiding this comment.
Cycle 3: prior blocking credential-leak issue in create_source is resolved — the enqueue body is now passed through redact_secret_fields before logging, masking credentials/rest_config/catalog_config (covering SQL passwords, filesystem aws_secret_access_key, REST bearer tokens, and Iceberg catalog tokens). Other body-carrying calls carry no secrets. LGTM.
…t/refresh
Collapse the sprawling 13-subcommand surface into the same shape as
`hotdata connections`, focused on a guided, catalog-driven onboarding
flow:
new interactive wizard, driven by the connector catalog
list the sources you've ingested (result databases)
create scriptable onboarding; `create list` browses the catalog
import SQL front-door (renamed from `query`); defaults to a
reasonable sample (SELECT * FROM <source> LIMIT 1000) when
no query is given
refresh re-drain + poll an ingest to completion
The `new` wizard is catalog-driven: the /ingest/connectors catalog
returns a `template` per REST service with base_url, auth shape, and
resources already filled in, so the wizard prompts only for the
`<PLACEHOLDER>` secrets — the user never types a URL the catalog knows.
Keyless services (auth "none") onboard with zero prompts.
Dropped as separate verbs: sql/rest/filesystem/iceberg (folded into
new/create by family), translate, connectors (→ `create list`),
status/drain (folded into the run+poll loop), schema/preview/download
(the read side is the core `query`/`databases`/`results` commands).
Client: ConnectorEntry now carries `auth` + `template`; dropped the
translate/schema/preview/download methods and types.
Catalog display drops redundant SQL dialect aliases (the server lists
both postgres/postgresql and mssql/sqlserver with identical labels):
the wizard and `create list` show the canonical name only, while
`create --service <alias>` still resolves against the full catalog.
Verified live against prod: `create list`, `list`, request assembly
from the catalog template, `import` default-query synthesis and
id-pinning, and `refresh` (drain + poll). A full `POST /sources`
onboard currently hangs server-side in the deploy path (unrelated to
the CLI — /queries, /drain, /jobs, /connectors all respond fast).
A slow enqueue (first-per-workspace runtime deploy) or a worker rollout makes the gateway hold the connection to its timeout, surfacing as the opaque 'error sending request'. Add a dark-grey hint pointing at the likely cause (service starting up / redeploying) and that a timed-out enqueue is safe to re-run.
`auth status` showed the first workspace from the live /workspaces API while `resolve_workspace` fell back to config.yml's first — so for a multi-workspace API key the status readout named a different workspace than commands used. Add one `credentials::default_workspace_id` helper (single-workspace token pins its own; multi/unrestricted key honors the saved default when it can reach it, else the credential's first authorized workspace) and use it from both paths. Drop the now-unused `config::resolve_workspace_id`.
…rce) The redundant postgres/postgresql and mssql/sqlserver catalog entries are now collapsed in dlthubworker's list_connectors, so the CLI no longer needs to dedupe on display. Keep only the family sort.
There was a problem hiding this comment.
Cycle 1–2 blocking issue resolved: create_source now redacts credentials/rest_config/catalog_config via redact_secret_fields before passing the body to --debug logging (src/client/ingest.rs:224, helper at :272), so source secrets no longer leak in plaintext. Verified create_query's unredacted body carries no credential fields.
1 participant
Adds a
hotdata ingestcommand group that drives the dlthubworker ingest API (/v1/ingest/*) through the gateway, with a UX modeled onhotdata connections.Command surface
The
newwizard is catalog-driven:/ingest/connectorsreturns atemplateper REST service withbase_url, auth shape, and resources pre-filled, so the wizard prompts only for the<PLACEHOLDER>secrets — the user never types a URL the catalog already knows. Keyless services onboard with zero prompts.importwith no SQL synthesizesSELECT * FROM <source> LIMIT 1000.Transport is a raw-HTTP client (
src/client/ingest.rs) since these routes aren't in the generated SDK yet.Auth model
create/import) uses the durablehd_API key as the bearer — the server 422s JWTs there (the drain job outlives them). Without a key the CLI fails fast with a--api-keyhint; read paths work on the session JWT.--debugredacts source secrets (credentials/rest_config/catalog_config) from the logged request body.error sending request.Also in this branch
auth statusreported the first workspace from the live/workspacesAPI whileresolve_workspacefell back toconfig.yml's first — so a multi-workspace API key made the status readout name a different workspace than commands actually used. Both now share onecredentials::default_workspace_idhelper.postgres/postgresql,mssql/sqlservercatalog entries) is fixed at the source in dlthubworker'slist_connectors— see the companion PR in that repo. This branch carries no display-side workaround.Testing
--debugredaction). Full suite green; clippy/fmt clean.create list,list, request assembly from catalog templates,importquery synthesis + id-pinning,refresh(drain + poll), and the workspace fix (auth statusnow matches theX-Workspace-Idcommands send). A fullPOST /sourcesonboard is slow (~190s) / occasionally exceeds the 300s gateway timeout during worker rollouts — a dlthubworker-side issue, not the CLI.