Skip to content

fix(api): pack cache keys with the full project id outside the vault namespaces #6166

Open

Description

@mmabrouk

What happens

_pack in api/oss/src/utils/caching.py truncates a project id to its last 12
characters before building the Redis key:

cache:p:{project_id[-12:]}:u:{user_id[-12:]}:{namespace}:{key}

Two projects whose UUIDs share that 12-character suffix therefore share every cache
entry in a namespace that does not opt out. PR #6164 added the full_project_id=True
opt-out and turned it on for the two vault namespaces (list_secrets,
list_secrets_generation), because those cache secret payloads. Everything else still
keys on the truncated id, including the two that decide authorization:

  • check_permissions (oss/src/apis/fastapi/access/router.py)
  • check_action_access (oss/src/services/db_manager.py,
    oss/src/core/access/permissions/service.py)
  • queries:retrieve, testsets:retrieve, artifact, and the webhook caches
  • the evaluation runtime locks (oss/src/core/evaluations/runtime/locks.py), which go
    through the same packing

Project ids are server-generated UUID4s, so a caller cannot steer a collision, and the
odds of one arising are remote. The consequence if it ever did — one project reading
another's cached permission result — is bad enough that the truncation should not be the
default.

Why it was not fixed in #6164

Flipping the default is a one-line change in _pack, but it is not a one-line change in
effect:

  1. invalidate_cache has no full_project_id parameter and packs its scan pattern with
    the truncated id. Flipping the writers without flipping invalidation would leave every
    namespace unable to clear its own entries.
  2. The evaluation runtime locks are packed the same way. Changing the key shape mid
    rolling deploy means old and new pods hold different keys for the same logical lock,
    so mutual exclusion is lost for the length of the deploy.
  3. Every namespace goes cold at once on deploy.

That is its own change with its own review, not a rider on a secrets PR.

What a fix needs

  • full_project_id plumbed through invalidate_cache (both the concrete-key and the
    pattern branch), then the default flipped for writers and invalidators together.
  • A deploy plan for the lock namespace (drain, or dual-read the old key shape for one
    release).
  • The tenancy test at api/oss/tests/pytest/unit/utils/test_cache_key_tenancy.py
    updated: it currently pins the truncated collision as a known hazard and points here.

Activity

  1. linear-code commented on Aug 21, 2026

    @linear-code
  2. mmabrouk commented on Aug 21, 2026

    @mmabrouk
    MemberAuthor

    Update: the vault list cache that originally motivated the full_project_id option was removed from PR #6164 (the list now reads the database on every request), so the specific blocker described above no longer exists. The platform-wide hazard remains as stated: authorization and permission caches key on a truncated project id, so two projects sharing a 12-character suffix would share cached results. The fix still needs the invalidation scan pattern and the lock keys to change together, as described.

  3. WhoamiI00 commented on Aug 25, 2026

    @WhoamiI00
    Contributor

    I'd like to take this one.

    Reading the current main, two of the three blockers in "What a fix needs" have dissolved, which makes this smaller than it looks:

    full_project_id no longer exists. It went away with the vault list cache in #6164 — grep -r full_project_id api/ returns nothing, and _pack truncates unconditionally today. So there is no flag to plumb through invalidate_cache: every writer, reader, and the pattern branch of invalidate_cache derive their key from the same _pack, so removing the truncation there flips them together by construction. Nothing parses a key back apart, and cache:p: is built in exactly one place.

    The tenancy test isn't there either — api/oss/tests/pytest/unit/utils/test_cache_key_tenancy.py doesn't exist on main (it presumably went with the same PR). So it's a new file rather than an update.

    That leaves the one real problem: the lock namespace across a rolling deploy. Taking the second of the two options you listed — dual-covering the old key shape for one release — locking.py is the only caller of _pack outside caching.py, and all five lock call sites (eval runtime, attachment sweep, account creation, and the EE spans/sessions/events/billing routers) funnel through its three functions, so the transition fits in one module and deletes cleanly next release. The ordering that makes it correct is claiming the legacy key first: a pod on the previous release only ever sets that one, so claiming it is what makes the two generations exclude each other.

    One coordination note: #6192 is open against the same _pack id-normalization block (wildcards for an omitted user_id under pattern=True). The two changes are orthogonal — I'll keep mine strictly to the truncation and rebase if that one lands first.

    Will open a PR shortly.

  4. added a commit that references this issue on Sep 5, 2026
    0cd64e1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugpythonPull requests that update Python code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions