feat(cloud): add managed control-plane funnel - #562
Open
dviejokfs wants to merge 86 commits into
Open
Conversation
Signed-off-by: David Viejo <dviejo@kfs.es>
Adds the temps-cloud crate (plugin, handler, service), extends the cloud client's link lifecycle and status, exposes a Cloud settings page in the console, and grows the CLI's cloud commands. Committed as-is from the worktree so main can be merged in with a real three-way merge rather than a stash replay. Signed-off-by: David Viejo <dviejo@kfs.es>
Conflicts, all in the console:
- App.tsx: main reindented the settings routes under a new wrapper; took
main's structure and re-added the cloud route.
- api/client/{index,sdk.gen,@tanstack/react-query.gen}.ts: both sides added
generated exports. Resolved by replaying each side's insertions onto the
base list rather than re-sorting, so the generator's own ordering is kept.
Regenerating against a live server is still the authority if these drift.
Signed-off-by: David Viejo <dviejo@kfs.es>
The sidebar entry existed but the palette did not, so ⌘K -- the way most people navigate the console -- could not reach managed cloud at all. Keywords cover what someone actually types when they are shopping for it (retention, backup, managed) rather than only the product name, and the e2e spec asserts both routes to it. Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
dviejokfs
force-pushed
the
feat/cloud-funnel
branch
from
August 6, 2026 09:53
a94be7a to
ee5beeb
Compare
📓 Changelog previewThis is what your commits will add to the generated ## [Unreleased]
### Added
- **cloud:** Add secure managed telemetry client
- **cloud:** Add managed cloud link lifecycle and console settings
- **console:** Find Temps Cloud from the command palette
- **cloud:** Integrate managed alert routing
- **cloud:** Declare backup restore artifacts
- **cloud:** Stream managed backups and AI access
- **cloud:** Report instance location metadata
- **multi-node:** Initialize and rotate cluster CA safely
- **security:** Harden cluster CA rotation
- **agent:** Add systemd service installer
- **cloud:** Auto-provision, rotate, and audit the managed backup credential
- **web:** Badge and guard Cloud-managed S3 sources in the UI
- **cloud-client:** Cloud telemetry read proxy client (spike, ADR-040)
- **otel:** ADR-040 Phase A - cloud telemetry fidelity tiers and backfill
- **otel:** ADR-041 - Cloud-primary telemetry writes
- **otel,cloud:** ADR-042 one-click Cloud telemetry activation
- **cloud:** Thread session tokens through managed backup credentials
### Build
- **mariadb:** Publish WAL-G image through GHCR
### Fixed
- **openapi:** Preserve canonical cloud schema diff
- **cloud:** Validate backup upload destinations
- **backups:** Make MariaDB WAL-G PITR E2E self-contained
- **cloud:** Harden managed data paths
- **cloud:** Preserve safe startup across restarts
- **cloud:** Persist in-flight telemetry across restarts
- **cloud:** Close security review findings
- **redis:** Report restore startup crash details
- **backups:** Reject deletion of retained sources
- **backups:** Strip Redis replication trailer
- **backups:** Pull digest-pinned sidecars
- **network:** Auto-detect underlay device instead of hardcoding eth0
- **e2e:** Provision immutable MariaDB recovery image
- **proxy:** Lower console fallback log severity
- **agent:** Derive overlay MTU from underlay
- **dns:** Avoid host resolver bind collision
- **network:** Join control plane to multi-node overlay
- **network:** Escape nftables baseline marker
- **network:** Enforce authoritative cluster CIDR
- **network:** Permit scoped VXLAN forwarding
- **network:** Match Docker VXLAN ingress path
- **network:** Gate control-plane kernel lifecycle test
- **cloud:** Restore cross-node services and MariaDB backups
- **network:** Preserve cross-node service return paths
- **network:** Reconcile upgraded firewall baselines
- **multi-node:** Finalize secure cluster onboarding
- **deployments:** Stop compose containers when deploy job fails
- **dns:** Prevent duplicate internal answers
- **multinode:** Route container lifecycle to owning worker
- **cloud-client:** Surface the real error while telemetry is dropping
- **cli:** Include temps-cloud crates in the server log level filter
- **config:** Preserve ADR-041/042 cloud fields not sent by every client
- **cloud-client:** Attribute health to a block reason instead of a placeholder
- **cloud:** Close credential-redaction and encoding gaps in session-token support
### Miscellaneous
- **merge:** Sync cloud funnel with main
- **api:** Regenerate Cloud integration clients
- **cloud:** Regenerate clients after main merge
- Add source attribution headers
- **web,cli:** Regenerate API clients and skill docs after merge
### Refactor
- **cloud:** Require backup checksums
- **deployments:** Centralize container runtime operations
### Testing
- **cloud:** Prove bounded shutdown flush
- **backups:** Prove MariaDB PITR with immutable images
- **cloud:** Fix migration and degraded-state regressions
- **redis:** Stabilize restart-loop regression
- **e2e:** Stabilize command and preview journeys
- **network:** Preserve existing control-plane routes |
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Resolve seven conflicts, six of which were artefacts rather than real
disagreements.
main canonicalised `apps/temps-cli/openapi.json` (recursively sorted keys)
while this branch added the `/cloud*` endpoints against the old key order.
Git's line-based merge therefore reported ~1,800 conflicting regions in a
93k-line generated file. Merging the parsed structure instead shows the true
change set is additive on both sides and disjoint: this branch adds 4 paths,
4 schemas and `AppSettings.cloud`; main adds 4 paths, 15 schemas and
`AppSettings.self_update`. Zero keys diverge. The resolved spec is main's
canonical file plus exactly those additions - 210 inserted lines, no
deletions.
The TypeScript clients are regenerated rather than hand-merged, since the
conflicts were single-line `import type {...}` / `export {...}` statements
listing every symbol in the API.
One trap worth recording: `web/`'s client could not simply be regenerated
from `openapi.json`. main's committed spec is stale - the ADR-036 sandbox
work landed `lifecycle`, `project_id` and `source_repo_url` in
`web/src/api/client/` (generated from a live server) without refreshing the
file. Regenerating from the file would have silently reverted those fields.
`web/` therefore keeps main's client with only the cloud delta applied, and
its diff against main is now purely additive.
Verified: cargo check --workspace --all-targets, cargo clippy -D warnings,
tsc --noEmit and eslint all clean.
Signed-off-by: David Viejo <dviejo@kfs.es>
Route managed AI requests through the linked Temps Cloud instance. Stream and retry WAL-G and native database backup artifacts, surface compatibility and WAL health in the OSS console, and include a repeatable backup stress harness. Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Route local, WAL-G, and native backup streams through one redirect-disabled uploader. Resolve and pin public object-storage addresses so unsafe DNS answers cannot receive backup data. Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Resolve the CI or local setup tag to Docker’s immutable image ID before exercising the production backup and restore configuration. Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
|
Too many files changed for review (293 files, 100 file limit). Bypass the limit by tagging |
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
dviejokfs
force-pushed
the
feat/cloud-funnel
branch
from
August 28, 2026 19:11
d8b82c0 to
1e82958
Compare
Initialize cluster trust before enrollment, require mTLS for new workers, expose the effective compute pool, and preserve explicit control-plane placement so existing clusters can be upgraded safely. Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
DeployComposeJob never overrode WorkflowTask::cleanup(), so when a compose deployment failed after containers were started (e.g. a later health-check job failing the workflow), the workflow gave up but the containers kept running under their `restart: unless-stopped`/`always` policy and crash-looped on the node indefinitely. DeployImageJob already tears down its container on cleanup(); mirror that by calling ComposeExecutor::teardown_at() for the compose project on failure. Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
MirrorHealth::Dropping carried only spooled/dropped counts. The flush error path discarded the actual failure (e.to_string()) the moment the spool overflowed, even though the sibling Buffering state already threaded that same error through correctly. An operator watching a degraded Cloud connection saw a generic "backend has been unreachable" message no matter what the real cause was. Dropping now carries a reason, populated from the failed delivery attempt, and health() preserves the last known reason across polls instead of discarding it when re-deriving state from the drop counters. Signed-off-by: David Viejo <dviejo@kfs.es>
…dential Linking a self-hosted instance to Temps Cloud previously started the backup mirror job but left the operator to configure their own S3 credentials by hand - identical to the free self-hosted experience, even on a paid plan whose pricing promises managed offsite backups. Add a `managed_by_cloud` flag to `s3_sources`, a `ManagedBackupCapability` wire type mirroring the existing `ManagedAiCapability` shape, and a `CloudClient::managed_backup_credentials` call so the OSS side can consume whatever real S3-compatible credentials Cloud's backend hands back. `CloudService::enroll` auto-provisions the managed row on link and `disconnect` removes it (unless still referenced by backup schedules or records, to avoid cascading away backup history). The row cannot be edited or deleted through the normal S3-source API - only Cloud's own disconnect path may remove it. Cloud-issued credentials are ephemeral by contract (expire at least daily), so `CloudService::start_backup_credential_rotation` runs a background loop that re-fetches and rotates the credential every 6 hours - a 4x safety margin - reusing the same provisioning path enrollment uses. If a rotation ever returns a different bucket than the one already on record (it never should, under a correct backend tenant->bucket contract), that's surfaced as a distinct `ProvisionedBucketChanged` outcome with a loud error log and its own audit action, instead of silently repointing the source at a bucket that doesn't have the operator's backup history. Cloud's backend endpoint itself does not exist yet (separate repo); this lands the OSS-side contract so it is ready to consume it. Signed-off-by: David Viejo <dviejo@kfs.es>
temps_cloud, temps_cloud_client, and temps_cloud_protocol were missing from the log-level allowlist, so every log line from the Cloud integration was silently dropped regardless of --log-level - including warnings from the backup mirror and, as of the managed-backup-source work, the loud error logged when a credential rotation lands on an unexpected bucket. Verified live: before this fix neither "Cloud backup mirror started" nor "Cloud backup credential rotation started" appeared in the boot log at any level; after it, both do. Signed-off-by: David Viejo <dviejo@kfs.es>
Regenerate the OpenAPI client so managed_by_cloud is a real typed field on S3SourceResponse (it previously wasn't - the existing is_default handling had to cast around the same gap). S3SourcesManagement now badges a managed row "Managed by Temps Cloud" and hides its Edit/Delete actions, matching the backend guards that already reject those requests for such a row - so an operator sees why the option is missing instead of hitting a 400 after clicking it. Signed-off-by: David Viejo <dviejo@kfs.es>
Route lifecycle, logs, exec, and terminal access through one location-aware interface so callers behave consistently for control-plane and worker containers. Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Adds CloudLink::clickhouse_query_client(), which builds a clickhouse::Client authenticated with the instance's existing enrollment token and pointed at Temps Cloud's new read-only ClickHouse wire-protocol proxy. Reuses the same storage types (ClickHouseOtelStorage/ClickHouseBackend) and query-building code the instance already has for local ClickHouse, instead of a new query API or trait, per ADR-040 section 2. Security-reviewed: no raw-credential duplication (single Authorization header only), wall-clock timeout matching every other Cloud call, narrowed public surface, gated behind the existing telemetry-export feature switch. Not wired into any read path yet — CloudRoutedOtelStorage/ CloudRoutedAnalyticsEvents, the SourceRouter, and TelemetrySourceBadge wiring are follow-up work (ADR-040 Phase B). TelemetrySourceBadge.tsx ships unwired, ready for that follow-up. ADR-040 also reconciles its originally-proposed bespoke REST API design with this wire-protocol-proxy approach, which a spike proved simpler, and redacts private temps-app implementation details in favor of contract-level descriptions. Signed-off-by: David Viejo <dviejo@kfs.es>
…fill Adds an opt-in per-project Queryable fidelity tier for Cloud-mirrored telemetry, alongside the existing default Metered tier (pseudonymized ids, constant span name, no attributes). Queryable ships real span names/ids, service name, span kind, status code, parent span id, environment, and an operator-curated exact-match attribute allowlist (empty/deny-by-default). Every failure path - missing project, DB error, unwired cache, corrupt column value - resolves to Metered, since that is the only recoverable direction for a one-way data-egress decision. Also adds `temps backfill cloud-telemetry`, a resumable, dry-run-capable CLI subcommand that ships a project's pre-existing local telemetry once its fidelity is raised (raising fidelity only affects spans ingested after the change, which would otherwise leave a permanent gap), plus a small Postgres- backed progress record, a read-only status endpoint, and a Console card so an operator can see backfill progress without watching a terminal - the Console only observes and instructs, it never triggers the backfill itself. Security-audited twice (initial pass plus a follow-up on the remediation diff). Both passes confirmed the properties that matter most - the Metered gate cannot leak real data, and the attribute allowlist is genuinely exact- match/deny-by-default - are sound, with no exploitable path found. Findings were LOW/INFO: a confusing CLI error for a nonexistent project, a missing audit-log entry for the backfill's data egress, an overstated privacy claim about project_ref pseudonymization (Cloud itself, as the HMAC key holder, can invert it - unlike the trace/span pseudonyms, which stay sound), and a test-hardening opportunity, all fixed. Policy-cache invalidation and spool purging on a fidelity downgrade are recorded as explicit Phase B requirements rather than built now, since the settings write path that would call them doesn't exist yet. Also extends _typos.toml with a synthetic parent-span-id test fixture the spell-checker flagged (one hex digit off the already-allowlisted W3C example span id, deliberately, so a span's own id and its parent's are never byte-identical in test assertions). Signed-off-by: David Viejo <dviejo@kfs.es>
Adds a Cloud-primary write mode for telemetry: once a project's fidelity tier is Queryable (ADR-040) and the ADR-0019 deletion endpoint is in place, spans can write directly to Temps Cloud via POST /v1/telemetry instead of the local span store (ClickHouse or TimescaleDB), removing the requirement for a local span store once Cloud is enabled. Phase B1: durable Postgres-backed span outbox (cloud_span_outbox), replacing the in-memory spool as the resilience mechanism for Cloud-primary writes. Load-tested to 500 spans/sec, drains to empty, survives a restart mid-outage, no whole-batch drops on handoff. Phase B2: write-mode switch (Local/Cloud) with an append-only interval ledger (project_telemetry_write_intervals) tracking history across disconnects/quota events/operator reversals; a read-routing decorator (CloudRoutedOtelStorage) installed at plugin registration so all 4 existing span readers (Traces page, HealthComputeService, CrossProjectTraceService, TraceReader/AI chat) stay correct without per-handler changes; automatic quota-exhaustion fallback to local writes; web UI and CLI surfaces for switching mode and viewing status. Security review (self-audit) found and fixed: - HIGH: consent could be bypassed via a two-step write-mode-then- fidelity change that let queued spans leave after downgrade, fixed by spilling unconditionally on Local and refusing fidelity downgrades while spans remain queued (409 FidelityDowngradeBlockedByQueuedSpans) - MEDIUM: cross-tenant project-id leak on the cloud-telemetry status endpoint, fixed with the existing hidden_project_ids() pattern - MEDIUM: outbox rows never purged on project deletion / dead letters retained forever, fixed with deletion-time purge and time-bounded dead-letter payload redaction - MEDIUM: a disconnect could strand queued spans under a specific interleaving, fixed in the drain/spill/flip ordering Also fixes a latent bug caught by an existing test: an out-of-range MIN_UTC sentinel used to mean "unbounded query" was being bound directly into the new ledger range query, silently making unbounded Cloud-primary trace queries return empty. Known follow-ups (not blocking): web Cloud settings page has no disconnect confirmation dialog yet (CLI has one); a few API/UI surface locations differ slightly from the ADR pending consolidation with concurrent backup-credential-rotation work; Phase C (export-back for logs/metrics/analytics) is deferred. See docs/adr/041-cloud-primary-telemetry-writes.md. Signed-off-by: David Viejo <dviejo@kfs.es>
Resolves conflicts between feat/cloud-funnel and the latest main: - crates/temps-migrations/src/migration/mod.rs: reorders the migration registry to include both main's m20260831_000001_create_analytics_ingest_keys and this branch's ADR-040/041 migrations (m20260901_000001..000006) in chronological order. - crates/temps-migrations/tests/migration_tests.rs: kept this branch's existing steps_back_to() helper (main independently added an equivalent inline fix for the same hardcoded-step-count bug; same semantics, this branch's version is reused since it's already a shared helper). - crates/temps-providers/src/externalsvc/mariadb.rs: both sides added unrelated, non-overlapping methods near the same anchor point (this branch's WAL-G binlog helpers vs. main's Compose execution-environment addressing); kept both. - crates/temps-providers/src/externalsvc/redis.rs: kept this branch's new restart-loop regression test. Dropped the now-dead `DEPLOYMENT_MODE_MUTEX` import: main removed the shared mutex and every DEPLOYMENT_MODE-env-mutating test that needed it (confirmed no remaining references anywhere in temps-providers), and this branch's new test doesn't use it either. - scripts/test-compose-security.sh: took main's version wholesale. Main rewrote this script extensively (Docker/Compose availability guards, ClickHouse credential rotation, workload/saturation probes) since this branch last synced; this branch's only change was a small Postgres health-check wait that predates that rewrite. - _typos.toml, Cargo.lock: straightforward union / regenerated. - apps/temps-cli/openapi.json, apps/temps-cli/src/api/*, web/src/api/client/*: regenerated from a server built off the merged source per the project's documented merge-conflict procedure for generated API clients, then typechecked both packages. - crates/temps-analytics/src/ingest_keys/test_fixtures.rs, crates/temps-analytics-performance/src/handlers/handler.rs: added the three cloud_telemetry_* fields to two projects::Model test fixtures main introduced after this branch's fields were added, caught by `cargo check --tests --workspace`. Verified: full workspace `cargo check --lib` and `cargo check --tests` clean, temps-migrations tests pass (including the hand-merged reversibility test), both apps/temps-cli and web typecheck clean against the regenerated clients. Signed-off-by: David Viejo <dviejo@kfs.es>
Add a bulk-job engine that switches many projects from local to Cloud-primary telemetry writes and backfills their pre-existing local history in one action, running in-process while temps serve stays live (no downtime, per ADR-042). - P0: give backfill submissions their own scope on CloudLink so a live mirror flush can't conflate counters with a concurrent backfill. - P1: cloud_telemetry_bulk_jobs / cloud_telemetry_bulk_job_projects tables, CloudBulkActivationWorker (sequential per-project switch then backfill, durable cursor, cancel, resume-on-restart). - P2: estimate/confirm HTTP endpoints with a signed plan_token, CLI parity (cloud telemetry bulk-switch/bulk-status/bulk-cancel), and a status-card UI with progress and ETA. - P3: auto-enqueue activation from POST /cloud/enroll so a Cloud purchase activates every eligible project without a manual step. - P4: a byte-budget anomaly guard that pauses a runaway project's backfill instead of shipping unbounded history. Security review flagged the P3 auto-spend path as unsafe as first implemented; hardening fixes for those findings are landing in this same change (see the ADR's Consequences/Risks sections). See docs/adr/042-one-click-cloud-telemetry-activation.md for the full design, alternatives considered, and open questions. Signed-off-by: David Viejo <dviejo@kfs.es>
Signed-off-by: David Viejo <dviejo@kfs.es>
Regenerate web/src/api/client from the merged server (ADR-042 endpoints plus everything origin/feat/cloud-funnel added since this branch diverged) and refresh the CLI skill command references so generateDocs' sync tests pass again. Signed-off-by: David Viejo <dviejo@kfs.es>
An unrelated PUT /settings save was silently resetting telemetry_bulk_anomaly_factor, telemetry_bulk_rate_limit_spans_per_sec, telemetry_outbox_max_bytes and backend_url to their defaults, because AppSettings deserializes an omitted `cloud` block to CloudSettings default rather than leaving it untouched. Worse than a silent reset: since the merge happened after the anomaly-factor widen/narrow check, an operator who had narrowed the guard got a spurious instance-admin-required 403 on every unrelated settings save (read as widening back to the default), and a successful admin save wrote a bogus guard-changed audit record for a change nobody made. Fix reads per-key presence off the raw request body before typed deserialization, so an explicit value (including null, to clear a field) still overrides the stored one, while an absent key preserves it. A value-equality heuristic would have made the compiled-in default unwritable, so presence is tracked instead. Also send the full cloud block from the console's own save path as defense in depth, even though the backend fix alone closes the hole for every caller. Signed-off-by: David Viejo <dviejo@kfs.es>
…aceholder
CloudLink::block_outbound() only recorded the block reason in
outbound_blocked_reason; it never touched self.health. Any span dropped
while outbound was blocked (e.g. startup reconciliation refusing an
origin change before the flusher's first tick ever runs) left health()
with no real Dropping/Buffering reason to report, so it fell back to
the generic "spans were discarded before a mirror delivery attempt
could report why" placeholder -- even when block_outbound was called
with a perfectly specific reason.
Found this debugging a real report: an instance's Cloud integration was
disabled at startup because its persisted link still pointed at a
stale local dev backend while its configured backend had changed to
production. The specific, actionable error ("Disconnect from
http://... before changing the managed backend to ...") was logged
server-side but never reached the operator-visible mirror health,
which only ever showed the generic placeholder no matter how many
times "Check again" was clicked.
block_outbound now sets health to Dropping (if anything has already
been dropped) or Buffering with the given reason, matching what a
failed flush() already does. Existing test coverage for the
"real gap in delivery history" case (a_saturated_ingest_queue_drops_...)
is unaffected since it never calls block_outbound.
Signed-off-by: David Viejo <dviejo@kfs.es>
A prefix-scoped backup credential is the only way to isolate tenants inside one shared bucket, because R2/S3 API tokens scope to a bucket and never to an object prefix. Such a credential can only be minted through a temporary-access API, and SigV4 rejects the resulting key pair unless a session token travels with it as `X-Amz-Security-Token`. Nothing in this repo had a session-token concept, so a credential vended that way was unusable end to end. Wire protocol: `ManagedBackupCapability` gains `session_token` and `expires_at`, both `Option` and `#[serde(default)]`, with `session_token` redacted by the existing custom `Debug` exactly like `secret_key`. Purely additive — a backend that sends neither field decodes to `None`/`None`. Storage: `s3_sources` gains a nullable `session_token` column, encrypted at rest through the same `EncryptionService` that already seals `access_key_id` and `secret_key`, plus a nullable `credentials_expire_at` so the console can state when a credential lapses instead of discovering it through a failed upload. `encrypt_credentials` seals the token only when present, so an operator-configured source stores NULL rather than an encrypted empty string; `decrypt_session_token` is the single read path and returns `None` for NULL and for the empty string. Signing and shell-outs: `S3Credentials` carries the token, and every `Credentials::new` built from an `s3_sources` row now passes it as the third argument instead of a hardcoded `None`. Every engine that shells out to `wal-g`/`mariabackup` appends `AWS_SESSION_TOKEN` via `aws_session_token_env`, which returns `None` for a long-lived credential so the variable is absent rather than empty — an empty one gets signed and rejected. `mc` reads no such variable, so its alias credentials take the token as a third colon-separated field (`mc_host_credential`), and the paths that configure an alias with `mc alias set` — which has no positional slot for a token — additionally emit an `MC_HOST_<alias>` override, but only when a token exists. `S3Credentials` and `S3SourceCredentials` also get hand-written `Debug` impls that redact both the secret key and the session token; the derived impls printed `secret_key` verbatim. A normal user-configured source is unaffected: `session_token` is `None` from the wire through the column, the signer and the container environment, and the API response type never carried it, so no route shape and no generated client changes. Prerequisite for Temps Cloud's managed backup credential vending; degrades safely until the control plane starts sending the field. Signed-off-by: David Viejo <dviejo@kfs.es>
…oken support
A follow-up to the session-token work on `s3_sources`, addressing findings from
a security review of that commit.
Redaction. `mc` echoes the credential-bearing `MC_HOST_*` URL it could not use
straight into stderr, and that stderr is logged, persisted into
`external_service_backups.error_message` / `restore_runs.error`, and returned
through the API. Two of the three redaction lists in `rustfs.rs` still held only
the four key/secret entries they had before a session token existed, so a
temporary destination or source credential leaked in full. `s3.rs` and
`s3_mirror.rs` had no redaction at all — pre-existing for a self-managed secret
key, but materially worse for a vended token that is not individually revocable
and whose only revocation is its 24h TTL.
`redact_sensitive_output` moves out of `rustfs.rs` into `externalsvc::mod`
alongside a new `SensitiveValues` builder, which is now the single place the
list is constructed. `SensitiveValues::credential` takes the session token as a
required argument, so a call site cannot silently omit it the way three ad-hoc
arrays could drift; it also registers the percent-encoded spelling of each
value, because that is the form mc prints back. Applied at all three `rustfs.rs`
sites and at the stderr-logging / error-string sites in `s3.rs` (`backup_to_s3`,
`restore_from_s3`, `restore_in_place`, `restore_to_new_service`) and
`s3_mirror.rs`.
URL encoding. `mc_host_credential` / `mc_host_alias_override` spliced raw
credentials into a URL's userinfo. STS session tokens routinely contain `/`,
which ends the URL authority, so every Cloud-linked mc invocation would have
failed with a parse error rather than anything actionable. Each field is now
percent-encoded with a deliberately minimal set (`% : / ? # @ [ ] space`, plus
non-ASCII): an access key or secret key from the usual base64/hex alphabet
encodes to the same bytes it always did, so no existing long-lived credential
changes, and a test pins that byte-for-byte.
Empty session tokens. `Some("")` — reachable only from a Cloud-vended wire
payload, never from an existing self-hosted row — is now normalised to `None`
where `capability.session_token` is first assigned, and again in
`encrypt_credentials`, which is the DB-write boundary and the only place that
can guarantee "NULL, not the ciphertext of an empty string" regardless of the
caller. `build_walg_env` gains the empty check its own comment already required,
matching `aws_session_token_env` and `mc_host_credential`.
Comments. `credentials_expire_at` is documented as write-only rather than as
something the console already shows or rotation already consults; neither is
true yet. `RestoreContext::s3_source` now mentions the third decrypted field.
`mc_host_alias_override` records a verified gap for the ADR author: with no
`--api` flag, `mc alias set` auto-probes the endpoint using the positional
credentials only and never reads `MC_HOST_*` while setting an alias, so a key
pair needing a session token fails there before the override is consulted.
Signed-off-by: David Viejo <dviejo@kfs.es>
managed_backup_setup_from_outcome discarded ManagedBackupOutcome::NotConfigured's reason and ManagedBackupOutcome::Unavailable's detail, always showing the same placeholder text regardless of why managed backups aren't available (no storage configured on Cloud, no entitlement, instance too old to sign the credential, a transient backend error, ...). Both reasons already come from operator-facing, secret-free error messages, so show them directly instead of flattening every case into one sentence. Signed-off-by: David Viejo <dviejo@kfs.es>
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.
Summary
Evidence
cargo fmt --all -- --check: passedcargo check --lib: passed (existing warnings only)cargo clippy --lib -- -D warnings: passed (existing dependency/profile warnings only)cargo test --lib -p temps-notifications: 121 passedcargo test -p temps-cloud-client --lib flusher::tests: 7 passed, including successful delivery, backend rejection retention, and timeout preservationcargo test --lib -p temps-cloud-client: 42 passedcargo test --lib -p temps-cloud: 13 passedcargo test -p temps-cloud-client backup_upload_never_follows_provider_redirects -- --nocapture: 1 passedcargo test -p temps-cloud-client production_backup_targets_require_https_public_hosts_and_safe_headers -- --nocapture: 1 passedcargo test -p temps-cloud-client resolved_backup_hosts_reject_private_or_mixed_dns_answers -- --nocapture: 1 passedcargo test -p temps-cloud-client backup_upload_recovers_each_network_boundary_without_changing_identity -- --nocapture: 1 passedbun run typecheckinapps/temps-cli: passedbun run buildinweb: passed (existing CSS import warnings)#alerts, and SMTP capture in one runSecurity review