PostHog / PostHog/duckgres

In-memory (plain CREATE SECRET) user secrets linger across sessions on a reused worker

Open
#766 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
180
Forks
18
Avg merge
10h 44m
Merged PRs (30d)
85

Description

Problem

Found while writing a customer-runnable test script for #752 against mw-prod-us: a plain (in-memory) CREATE SECRET issued in session A is still visible in session B whenever B reuses the same hot-idle worker — empirically reproduced on a prod duckling. Since hot-idle reuse spans users of the same org, user A's in-memory secret (with live credential material) is readable by user B via duckdb_secrets() and usable by scope match.

This is pre-existing behavior, not a #752 regression: DuckDB secrets are instance-global, and the #752 session-start hygiene wipe deliberately clears only persistent-storage secrets, because the system catalog secrets (ducklake_s3, iceberg_sigv4, iceberg_oauth) are in-memory and must survive. Before #752 the same lingering applied to persistent secrets too (worse); the in-memory half of the gap remains.

Reproduce

psql ... -c "CREATE SECRET t1 (TYPE s3, KEY_ID 'x', SECRET 'y')"
psql ... -c "SELECT name FROM duckdb_secrets() WHERE name='t1'"   # new session, reused worker → t1

Proposed fix

Extend wipeUserPersistentSecrets (duckdbservice/user_secrets.go) into a full user-secret wipe at session create: drop all secrets — any storage — whose name is not reserved (usersecrets.IsReservedName covers the system set + __default_*/duckgres_* prefixes). The reserved-name guard already exists and is already load-bearing for CREATE interception; reusing it here keeps one source of truth. Same one-session-per-worker precondition assertion applies.

Caveat to check: unnamed user secrets get DuckDB default names (__default_s3), which collide with the reserved prefix — wiping them is correct (they're user-created), so the wipe predicate should be "not a system secret name" rather than IsReservedName verbatim; may need to split the reserved set (system names vs reserved-for-create prefixes).

The e2e persistent_user_secret_isolation assertion in tests/e2e-mw-dev/harness.sh should gain a temp-secret leg (user A CREATE SECRET; user B must not see it).

🤖 Generated with Claude Code

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in duckdbservice/user_secrets.go by reviewing wipeUserPersistentSecrets and the existing usersecrets.IsReservedName handling, then inspect the persistent_user_secret_isolation assertion in tests/e2e-mw-dev/harness.sh. Confirm the distinction between system secrets and user-created default-named secrets, and extend the e2e coverage so a secret created by user A is absent for user B on a reused worker.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.