fix(data-warehouse): redact API key credentials from request logs

[MarconLP]

Problem

The Custom REST source lets the manifest configure API-key auth injected into a query parameter (or a custom header/cookie) under an arbitrary, user-chosen name. The tracked HTTP transport only redacts credentials from logs and captured request samples by matching against a fixed denylist of parameter/header names (url_utils._REDACT_PARAM_NAMES, sampling._AUTH_HEADER_NAMES). A manifest naming its key something off the denylist — e.g. subscription-key — would write the upstream API key in plaintext into data_imports.http.request log lines and into sampled requests in object storage.

Flagged in review of #59612.

Changes

Adds value-based redaction on top of the existing name-based scrubbers, mirroring how Airbyte masks airbyte_secret config values: we know the exact credential string, so we mask it wherever it appears regardless of injection location (query, header, cookie, body) or parameter name.

• url_utils.redact_literal_values() — replaces a known secret string and its URL-encoded forms with REDACTED. Skips empty/very short values so unrelated log text isn't mangled.
• auth.auth_secret_values() — extracts the credential strings (token / api_key / password) carried by an auth object.
• Threaded the values through make_tracked_session → TrackedHTTPAdapter → record_request, where they're applied to the logged URL and forwarded into the captured sample.
• Wired in automatically at both choke points:
  • Sync path: RESTClient derives the values from its auth object — so this protects every REST-based source, not just the custom one.
  • Probe path: the custom source's create-time validate_credentials probe registers the same values.

This is purely additive — redact_values defaults to empty, so behaviour is unchanged for callers that don't pass credentials.

How did you test this code?

I'm an agent; I did not perform manual testing. Added and ran automated tests (all passing):

• redact_literal_values — raw value, percent-encoded form, header/payload string, multiple secrets, short/empty skip, no-op when empty.
• auth_secret_values — extraction across bearer / api_key / http_basic, and empty when no credential set.
• Transport — adapter forwards redact_values to record_request (and defaults to empty).
• RESTClient — builds its tracked session with credentials derived from auth.
• Custom source probe — registers the credential for redaction when auth is injected into a query param.
• Observer end-to-end — a key in a non-denylisted query param (subscription-key) is masked in the logged URL while unrelated params survive.

🤖 Agent context

Authored by Claude Code (claude-opus-4-8) while working through review feedback on #59612. Tools: Read/Edit/Bash/Grep, plus two parallel Explore subagents to study how Airbyte handles credential masking and descending-order incremental resume.

Key decision: the reviewer offered "constrain the param name to the denylist" vs "thread the configured name into the scrubber." The Airbyte exploration showed the robust industry approach is value-based masking (mask the known secret value everywhere) rather than name-based, so we chose that — it covers any injection location and any name without restricting what users can name their auth param. Deriving the values from the auth object inside RESTClient (rather than threading them per-source) means the protection is automatic for all sources.

Stacked on #59612.

[MarconLP]

• fix(data-warehouse): re-enter credentials on custom source host change #60609
• fix(data-warehouse): redact API key credentials from request logs #60599 👈 (View in Graphite)
• fix(data-warehouse): correct custom source incremental sync and validation #59612 : 3 other dependent PRs (#59387 , #59525 , #60598 )
• feat(data-warehouse): add Custom REST source backend #59386
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
posthog/temporal/data_imports/sources/common/http/transport.py:86
The `getattr` fallback here is superfluous — `_redact_values` is always assigned in `__init__`, so the default `()` can never be reached. Direct attribute access is clearer and avoids the implication that the attribute might be absent.

```suggestion
                    redact_values=self._redact_values,
```

### Issue 2 of 2
posthog/temporal/data_imports/sources/common/rest_source/tests/test_rest_client.py:114-128
These two tests cover the same code path with different inputs and assertions. The project prefers parametrized tests — collapsing them removes the repeated `@patch` boilerplate and makes it easy to add further auth types in future without writing another method.

```suggestion
    @pytest.mark.parametrize(
        "auth,expected_redact_values",
        [
            (APIKeyAuth(api_key="sk_live_x", name="key", location="query"), ("sk_live_x",)),
            (None, ()),
        ],
    )
    @patch("posthog.temporal.data_imports.sources.common.rest_source.rest_client.make_tracked_session")
    def test_builds_session_with_credentials_for_redaction(self, MockSession, auth, expected_redact_values) -> None:
        MockSession.return_value.headers = {}
        RESTClient(base_url="https://api.example.com", auth=auth)
        assert MockSession.call_args.kwargs["redact_values"] == expected_redact_values
```

_{Reviews (1): Last reviewed commit: "fix(data-warehouse): redact API key cred..." | Re-trigger Greptile}

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-06-01 09:29 UTC	Run
prod-us	✅ Deployed	2026-06-01 09:41 UTC	Run
prod-eu	✅ Deployed	2026-06-01 09:42 UTC	Run