Skip to content

Sensitive ingest tokens - #121

Open
pawcik wants to merge 3 commits into
BetterStackHQ:mainfrom
pawcik:sensitive-ingest-tokens
Open

Sensitive ingest tokens#121
pawcik wants to merge 3 commits into
BetterStackHQ:mainfrom
pawcik:sensitive-ingest-tokens

Conversation

@pawcik

@pawcik pawcik commented Jul 20, 2026

Copy link
Copy Markdown

Summary

Three related hardening fixes around secret handling and API error behavior.

1. Mark ingest tokens as Sensitive

The computed token attributes on logtail_source and logtail_errors_application authenticate data ingestion into the team's account, but were not marked Sensitive. Terraform printed them in plaintext in every plan/apply — leaking them into CI logs, plan-bot PR comments (Atlantis, Spacelift, etc.), and terminal scrollback. Anyone reading those logs can inject arbitrary data into the sources.

The equivalent logtail_collector.secret was already Sensitive: true, so this closes an inconsistency rather than changing a design decision. The corresponding data sources pick the flag up automatically (they build their schemas by copying the resource schemas). The new js_tag_token stays non-sensitive, since it is a public browser-side token by design.

2. Gate API response bodies out of error diagnostics

API error responses were embedded verbatim in user-facing diagnostics (diag.Errorf("POST ... returned %d: %s", ..., body)). If a validation error echoes submitted fields — requests carry secret_access_key, database password, connection password — the secret lands in plaintext in terraform output and CI logs. Notably, this path was not covered by the existing TF_PROVIDER_LOGTAIL_LOG_INSECURE gate, which only controls debug logging.

Error diagnostics now include the response body only when TF_PROVIDER_LOGTAIL_LOG_INSECURE=1 is set — reusing the same opt-in that already governs request/response body logging in main.go, so there is a single, deterministic switch for "may print API payloads". The default error message tells the user how to opt in when debugging.

3. Stop retrying POSTs on 5xx

The retry policy retried POST requests on server errors. A create that succeeded server-side before the error response (e.g. a gateway timeout) would be retried and silently create a duplicate resource. POSTs now retry only on 429 rate limits; GET/PATCH/DELETE keep the default retry policy.

Other changes

  • examples/outputs.tflogtail_source_token output marked sensitive = true (required by Terraform ≥0.14 once the attribute is sensitive; the E2E output -json verification still sees the value, so CI checks keep working)
  • docs/ — regenerated via make gen
  • MakefileVERSION bumped to 10.15.12 (next patch after v10.15.11)
  • New unit tests for the error-body gate and the retry policy; TestResourceMetricPatchErrorPropagates now opts into the gate via t.Setenv to keep verifying body propagation

Impact on users

No resource changes or replacements — only display and error-handling behavior:

  • Plans show token = (sensitive value) instead of the plaintext token; users outputting these tokens directly need sensitive = true on their outputs (Terraform reports this with a clear error)
  • API error messages no longer include the raw response body by default; set TF_PROVIDER_LOGTAIL_LOG_INSECURE=1 to restore it when debugging

Testing

  • go test -timeout 10m ./... passes (acceptance-style provider tests + new unit tests)
  • make gen produces a clean diff (checked by the check_docs CI job)

🤖 Generated with Claude Code

Pawel Jedruch and others added 3 commits July 20, 2026 10:03
The computed token attributes of logtail_source and
logtail_errors_application authenticate data ingestion, but were not
marked Sensitive, so terraform plan/apply printed them in plaintext -
leaking them into CI logs and plan-bot PR comments. Mark them
Sensitive: true (this propagates to the corresponding data sources,
which copy the resource schemas) and mark the basic example's
logtail_source_token output sensitive so plans keep working.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
API error responses were embedded verbatim in user-facing diagnostics,
so a validation error echoing a submitted secret (bucket keys, database
passwords) would land in plaintext in terraform output and CI logs.
Include the response body in diagnostics only when the user has opted
into insecure logging via TF_PROVIDER_LOGTAIL_LOG_INSECURE=1 - the same
gate that already controls request/response body logging in main.go.
The default message tells the user how to opt in when debugging.

Also stop retrying POST requests on 5xx responses: the create may have
succeeded server-side before the error response, and retrying it would
silently create a duplicate resource. 429s are still retried.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pawcik

pawcik commented Jul 20, 2026

Copy link
Copy Markdown
Author

Follow-up: complete duplicate-create protection via Idempotency-Key

The "no POST retries on 5xx" change in this PR closes the most common duplicate-create trigger (a gateway timeout returning 502/504 after the create already landed), but it's worth noting what it deliberately doesn't cover, and what a complete fix would look like.

Current state of the stack (verified while preparing this PR):

  • The provider sends no idempotency mechanism — only Authorization, User-Agent, and Content-Type headers
  • go-retryablehttp's default policy is method-agnostic, and still retries POSTs on transport-level errors (e.g. a client-side timeout after the request was sent), because in that path CheckRetry receives a nil response and can't see the method
  • The API has no server-side dedup for most resources — as the CI comment in test.yml puts it, "duplicate names are allowed"; only errors_application and errors_application_group enforce unique names, and that's a business rule rather than an idempotency design

Suggestion: support an Idempotency-Key request header on the create endpoints (server-side), and have the provider generate one per logical create and reuse it across retries. That would make every retry — 5xx and transport error — safe, and would benefit all API clients, not just Terraform. Until then, the conservative retry policy in this PR is the practical guard.


Note: this PR was created during the Critique step of the Better Stack interview process. Feel free to use, adapt, or merge any part of it as you see fit — however, I won't be actively following up on this PR, so please treat it as a handoff rather than an ongoing contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant