confetti / confetti/confetti-node

Module-global yayson Store leaks records between requests (cross-tenant in multi-key processes)

Open Beginner friendly
#34 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
0
Forks
0
Avg merge
1h 51m
Merged PRs (30d)
2

Description

Maintainers: if you consider this a security issue, it may be worth converting to a security advisory. I only have read access so I could not open one. Impact is limited to consumers that use more than one API key in a single process — but that is exactly the shape of an API proxy or MCP server.

What happens

src/adapter.ts builds the deserializer once at module scope:

const { Store } = yayson()
const store = new Store()   // module-level, shared by every request in the process

and calls store.sync(body) for every response. yayson's Store accumulates records for the life of the process, keyed only by (type, id), and resolves relationship references by scanning all previously synced records.

So when response B contains a relationship reference — say { type: 'workspaces', id: '99' } — without an accompanying included block, yayson resolves it from whatever {workspaces, 99} some earlier request synced. If those requests used different API keys, one caller's record is spliced into another's result.

Reproduction

import nock from 'nock'
import Confetti from 'confetti'

const API = 'https://api.confetti.events'

// Caller A fetches event 7, which includes workspace 99.
nock(API).get('/events/7').query(true).reply(200, {
  data: { id: '7', type: 'events', attributes: { name: 'A-EVENT' },
          relationships: { workspace: { data: { id: '99', type: 'workspaces' } } } },
  included: [{ id: '99', type: 'workspaces', attributes: { name: 'TENANT-A-PRIVATE' } }],
}, { 'content-type': 'application/json' })
await Confetti.events.find(7, { apiKey: 'key-A' })

// Caller B fetches event 8, which references workspace 99 but includes nothing.
nock(API).get('/events/8').query(true).reply(200, {
  data: { id: '8', type: 'events', attributes: { name: 'B-EVENT' },
          relationships: { workspace: { data: { id: '99', type: 'workspaces' } } } },
}, { 'content-type': 'application/json' })
const b = await Confetti.events.find(8, { apiKey: 'key-B' })

console.log(b.workspace?.name)   // => "TENANT-A-PRIVATE"

Plain attributes do not bleed — only relationship resolution does. A control sync into a fresh Store returns the bare { id: '99' } stub, which confirms the extra data came from the shared store.

Impact

  • Correctness, always: relationship data can come from an earlier, unrelated response, so identical calls return different results depending on process history.
  • Isolation, in multi-key processes: any service that passes a per-request apiKey — the pattern the static methods exist to support — can return one caller's record to another.
  • Memory: the store is never reset or bounded, so a long-lived process accumulates every record it has ever seen, and sync cost grows with it.

Whether ids collide across accounts decides how reachable the cross-tenant case is. I could not verify how the API allocates ids; sample fixtures suggest large global sequences, which would make collisions unlikely. The correctness and memory halves hold regardless.

Suggested fix

Construct the store per call (or store.reset() before each sync) in httpRequest:

const store = new Store()   // inside httpRequest, not module scope

A per-call store costs an object allocation per request and removes the shared state entirely.

Workaround for consumers

Pass raw: true and flatten with your own new Store() per call. That is what we ended up doing in an MCP server built on this package.

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 src/adapter.ts at the module-level Store and follow its use in httpRequest, then run the two-key reproduction from the issue. Confirm that relationship references no longer resolve from records synced by an earlier request, while the existing response deserialization still works without accumulating process-wide state.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.