Restore custom taint map from cache; make taint analysis deterministic

[danog]

Problem

Custom taint types (registered via Codebase::getOrRegisterTaint() — e.g. from @psalm-taint-sink / @psalm-taint-source docblocks or a plugin) are assigned a bit lazily, in the order they are first encountered while scanning ($id = 1 << $this->taint_count++). That bit is then baked into the cached FileStorage/ClassLikeStorage (a parameter's sinks, a function's taint_source_types, etc.).

On a cache hit the defining docblocks are not re-parsed, so getOrRegisterTaint() never runs and Codebase::$custom_taints ends up empty (or ordered differently than the run that wrote the cache). The taint bits baked into the reused storage then no longer correspond to the taint names registered in the current run, so cached sinks and sources silently stop matching and no taint issues are reported — unless caching is disabled with --no-cache, which forces a full re-scan.

This is easy to hit with a project that defines a custom taint on a vendored/rarely-changed class (a Mongo/NoSQL sink, say): the sink lives in a file that is always served from cache, so after the first run its taint bit is effectively dead and the vulnerability stops being reported.

Fix

Persist the custom taint name → bit map next to the cache (ProjectCacheProvider::save/loadCustomTaints()) and restore it before anything is scanned (ProjectAnalyzer), so the bits baked into reused storage stay valid across runs. The map only ever grows, so previously-cached storage remains consistent. It is saved after every run that populated the cache (not just full-project runs), because individual-file and diff runs also write taint bits into the storage cache. With --no-cache there is no cache provider, so nothing is persisted and each run re-registers from scratch as before.

Single-process taint registration

Scanning — and therefore docblock taint registration — runs in forked worker processes, and each worker assigns bits from its own counter. So two brand-new custom taints first discovered by different workers would both get the first free bit (or one taint would get two different bits depending on discovery order), corrupting the merged storage and the findings built from it. (Thanks to the Cursor Bugbot review for flagging this.)

Rather than a best-effort post-scan merge, new taints are now registered in the parent's single authoritative registry: a worker sends the taint name over its task channel and the parent replies with the canonical bit (Codebase::registerTaintFromWorker(), driven by a small message pump added to Pool::run). Every worker therefore agrees on the bit for a given name, and the parent map is complete so it can be persisted directly.

Determinism

array_map() / array_filter() named their synthetic offset / method-call variables with mt_rand(), and those names become part of taint-graph node ids, so the taint graph varied between runs. They now derive a deterministic discriminator from the call position — the $fake_var_discriminator "known value" hook that already existed but was unused by default. (This doesn't change issue counts — the random id is internally consistent within a run — but it makes the taint graph byte-reproducible.)

Tests

• tests/Cache/CustomTaintCacheTest.php — export/import round-trip preserves bits and appends new taints; import is a no-op once taints are registered; a forked worker registers taints through the parent registry; two workers with opposite discovery order agree on every taint's bit (the collision case); and custom taint bits survive cache reuse across runs.
• tests/TaintDeterminismTest.php — the same array_map() / array_filter() code analysed twice yields an identical taint graph (fails against mt_rand()).

Existing TaintTest, ReturnTypeProvider/ArrayMapTest, ArrayFilterTest and cache tests still pass. Validated end-to-end on a large real project that forks for scanning: cold and warm runs report the same taints and persist a complete, stable taint map.

🤖 Generated with Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[danog]

Good catch on the forked-scan taint merge — that gap is now fixed: ShutdownScannerTask returns each worker's custom_taints/taint_count, and Scanner folds them into the parent Codebase via the new Codebase::mergeCustomTaints() while collecting scan results, so the persisted map covers every taint baked into the merged storage. Since workers fork from the parent (sharing its restored map), a given name resolves to the same bit everywhere, so the union is safe. Covered by testMergeFoldsInTaintsRegisteredByForkedScanWorkers.

bugbot run

[danog]

bugbot run

[cursor]

Stale Bugbot comment from a previous run.

[danog]

Re-worked the forked-scan taint handling per the discussion: instead of a post-scan merge (which, as noted, can't reconcile two workers that assigned the same bit to different taints), new custom taints are now registered in the parent's single authoritative registry. A worker sends the taint name over its Amp task channel and the parent replies with the canonical bit (Codebase::registerTaintFromWorker(), driven by a message pump added to Pool::run), so every worker agrees and no bit can be double-assigned. Covered by testForkedWorkerRegistersTaintsThroughParentRegistry and testConcurrentWorkersAgreeOnTaintBits (two workers, opposite discovery order), and validated end-to-end on a project that forks for scanning.

bugbot run

[danog]

bugbot review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 4d09331. Configure here.}