[Flight] Taint APIs (#27445) This lets a registered object or value be "tainted", which we block from crossing the serialization boundary. It's only allowed to stay in-memory. This is an extra layer of protection against mistakes of transferring d - #12
Open
Menkalian wants to merge 1 commit into
Conversation
This lets a registered object or value be "tainted", which we block from crossing the serialization boundary. It's only allowed to stay in-memory. This is an extra layer of protection against mistakes of transferring data from a data access layer to a client. It doesn't provide perfect protection, because it doesn't trace through derived values and substrings. So it shouldn't be used as the only security layer but more layers are better. `taintObjectReference` is for specific object instances, not any nested objects or values inside that object. It's useful to avoid specific objects from getting passed as is. It ensures that you don't accidentally leak values in a specific context. It can be for security reasons like tokens, privacy reasons like personal data or performance reasons like avoiding passing large objects over the wire. It might be privacy violation to leak the age of a specific user, but the number itself isn't blocked in any other context. As soon as the value is extracted and passed specifically without the object, it can therefore leak. `taintUniqueValue` is useful for high entropy values such as hashes, tokens or crypto keys that are very unique values. In that case it can be useful to taint the actual primitive values themselves. These can be encoded as a string, bigint or typed array. We don't currently check for this value in a substring or inside other typed arrays. Since values can be created from different sources they don't just follow garbage collection. In this case an additional object must be provided that defines the life time of this value for how long it should be blocked. It can be `globalThis` for essentially forever, but that risks leaking memory for ever when you're dealing with dynamic values like reading a token from a database. So in that case the idea is that you pass the object that might end up in cache. A request is the only thing that is expected to do any work. The principle is that you can derive values from out of a tainted entry during a request. Including stashing it in a per request cache. What you can't do is store a derived value in a global module level cache. At least not without also tainting the object.
There was a problem hiding this comment.
Pull request overview
Adds experimental “taint” APIs to React Flight/Server Components to prevent specific in-memory values (objects, functions, strings, bigints, and certain binary payloads) from crossing the serialization boundary to the client.
Changes:
- Introduces
experimental_taintUniqueValueandexperimental_taintObjectReferenceand a shared taint registry backed byWeakMap/MapplusFinalizationRegistryintegration. - Enforces taint violations during Flight serialization on the server (including optional binary-flight checking).
- Adds feature flag plumbing, error codes, ESLint/Flow environment updates, and coverage tests for taint behavior.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/rollup/validate/eslintrc.umd.js | Allows FinalizationRegistry as a readonly global for UMD validation. |
| scripts/rollup/validate/eslintrc.rn.js | Allows FinalizationRegistry as a readonly global for RN validation. |
| scripts/rollup/validate/eslintrc.fb.js | Allows FinalizationRegistry as a readonly global for FB validation. |
| scripts/rollup/validate/eslintrc.esm.js | Allows FinalizationRegistry as a readonly global for ESM validation. |
| scripts/rollup/validate/eslintrc.cjs2015.js | Allows FinalizationRegistry as a readonly global for CJS2015 validation. |
| scripts/rollup/validate/eslintrc.cjs.js | Allows FinalizationRegistry as a readonly global for CJS validation. |
| scripts/flow/environment.js | Declares FinalizationRegistry for Flow typechecking. |
| scripts/error-codes/codes.json | Adds new error-code messages related to taint API misuse. |
| packages/shared/forks/ReactFeatureFlags.www.js | Adds enableTaint flag (disabled for www fork). |
| packages/shared/forks/ReactFeatureFlags.test-renderer.www.js | Enables enableTaint for test-renderer www fork. |
| packages/shared/forks/ReactFeatureFlags.test-renderer.native.js | Enables enableTaint for test-renderer native fork. |
| packages/shared/forks/ReactFeatureFlags.test-renderer.js | Enables enableTaint for test-renderer fork. |
| packages/shared/forks/ReactFeatureFlags.native-oss.js | Enables enableTaint for native OSS fork. |
| packages/shared/forks/ReactFeatureFlags.native-fb.js | Enables enableTaint for native FB fork. |
| packages/shared/binaryToComparableString.js | Adds helper to convert binary views to comparable strings for registry lookups. |
| packages/shared/ReactFeatureFlags.js | Adds enableTaint to the main feature flag list. |
| packages/react/src/ReactTaintRegistry.js | Introduces taint registries for object and value taint tracking. |
| packages/react/src/ReactTaint.js | Implements taint APIs and FinalizationRegistry-driven cleanup bookkeeping. |
| packages/react/src/ReactSharedSubset.experimental.js | Exposes server-only experimental taint APIs from the React experimental subset. |
| packages/react/src/ReactServerSharedInternals.js | Plumbs taint registries into server shared internals behind enableTaint. |
| packages/react-server/src/ReactFlightServer.js | Enforces taint checks during Flight serialization and manages per-request taint cleanup queues. |
| packages/react-client/src/tests/ReactFlight-test.js | Adds tests validating taint violations for objects/functions/primitives/binary and request-lifetime semantics. |
| .eslintrc.js | Allows FinalizationRegistry as a readonly global for the repo ESLint config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+10
to
+11
| // Turns a TypedArray or ArrayBuffer into a string that can be used for comparison | ||
| // in a Map to see if the bytes are the same. |
There was a problem hiding this comment.
The header comment says this accepts a "TypedArray or ArrayBuffer", but the function signature is $ArrayBufferView and the implementation relies on .buffer/.byteOffset. Passing an ArrayBuffer would break; update the comment (or broaden the signature/implementation if ArrayBuffer support is intended).
Suggested change
| // Turns a TypedArray or ArrayBuffer into a string that can be used for comparison | |
| // in a Map to see if the bytes are the same. | |
| // Turns an ArrayBufferView (e.g. a TypedArray or DataView) into a string that can | |
| // be used for comparison in a Map to see if the bytes are the same. |
Comment on lines
+15
to
+18
| return String.fromCharCode.apply( | ||
| String, | ||
| new Uint8Array(view.buffer, view.byteOffset, view.byteLength), | ||
| ); |
There was a problem hiding this comment.
String.fromCharCode.apply with a Uint8Array can throw (or be extremely slow) for larger buffers due to engine argument limits. Consider a chunked conversion approach (or another stable hashing/encoding strategy) so taint checks can handle large typed arrays safely.
Suggested change
| return String.fromCharCode.apply( | |
| String, | |
| new Uint8Array(view.buffer, view.byteOffset, view.byteLength), | |
| ); | |
| const uint8 = new Uint8Array(view.buffer, view.byteOffset, view.byteLength); | |
| // For small buffers, keep the simple fast path. | |
| if (uint8.length <= 0x8000) { | |
| return String.fromCharCode.apply(String, uint8); | |
| } | |
| // For larger buffers, convert in chunks to avoid exceeding engine argument limits. | |
| let result = ''; | |
| const CHUNK_SIZE = 0x8000; | |
| for (let i = 0; i < uint8.length; i += CHUNK_SIZE) { | |
| const chunk = uint8.subarray(i, i + CHUNK_SIZE); | |
| result += String.fromCharCode.apply(String, chunk); | |
| } | |
| return result; |
Comment on lines
+19
to
+20
| // Byte lengths of all binary values we've ever seen. We don't both refcounting this. | ||
| // We expect to see only a few lengths here such as the length of token. |
There was a problem hiding this comment.
Typo/grammar in the comment: "We don't both refcounting this" should be "We don't bother refcounting this" (and "the length of token" → "the length of a token").
Suggested change
| // Byte lengths of all binary values we've ever seen. We don't both refcounting this. | |
| // We expect to see only a few lengths here such as the length of token. | |
| // Byte lengths of all binary values we've ever seen. We don't bother refcounting this. | |
| // We expect to see only a few lengths here such as the length of a token. |
| } | ||
| } | ||
|
|
||
| // If FinalizationRegistry doesn't exist, we assume that objects life forever. |
There was a problem hiding this comment.
Typo in the comment: "we assume that objects life forever" should be "we assume that objects live forever".
Suggested change
| // If FinalizationRegistry doesn't exist, we assume that objects life forever. | |
| // If FinalizationRegistry doesn't exist, we assume that objects live forever. |
Comment on lines
+13
to
+25
| const heldValues = []; | ||
| let finalizationCallback; | ||
| function FinalizationRegistryMock(callback) { | ||
| finalizationCallback = callback; | ||
| } | ||
| FinalizationRegistryMock.prototype.register = function (target, heldValue) { | ||
| heldValues.push(heldValue); | ||
| }; | ||
| global.FinalizationRegistry = FinalizationRegistryMock; | ||
|
|
||
| function gc() { | ||
| for (let i = 0; i < heldValues.length; i++) { | ||
| finalizationCallback(heldValues[i]); |
There was a problem hiding this comment.
The FinalizationRegistry mock and heldValues array are defined at module scope and never reset. Because this file calls jest.resetModules() in beforeEach, finalizationCallback will be replaced across tests while heldValues persists, which can cause cross-test interference (e.g. stale held values being finalized by a newer callback). Consider moving the mock setup into beforeEach/beforeAll and clearing/restoring state in afterEach/afterAll (including resetting heldValues and restoring the original global.FinalizationRegistry).
Suggested change
| const heldValues = []; | |
| let finalizationCallback; | |
| function FinalizationRegistryMock(callback) { | |
| finalizationCallback = callback; | |
| } | |
| FinalizationRegistryMock.prototype.register = function (target, heldValue) { | |
| heldValues.push(heldValue); | |
| }; | |
| global.FinalizationRegistry = FinalizationRegistryMock; | |
| function gc() { | |
| for (let i = 0; i < heldValues.length; i++) { | |
| finalizationCallback(heldValues[i]); | |
| let heldValues = []; | |
| let finalizationCallback; | |
| const originalFinalizationRegistry = global.FinalizationRegistry; | |
| function FinalizationRegistryMock(callback) { | |
| finalizationCallback = callback; | |
| } | |
| FinalizationRegistryMock.prototype.register = function (target, heldValue) { | |
| heldValues.push(heldValue); | |
| }; | |
| beforeAll(() => { | |
| global.FinalizationRegistry = FinalizationRegistryMock; | |
| }); | |
| beforeEach(() => { | |
| heldValues = []; | |
| finalizationCallback = undefined; | |
| }); | |
| afterAll(() => { | |
| global.FinalizationRegistry = originalFinalizationRegistry; | |
| }); | |
| function gc() { | |
| for (let i = 0; i < heldValues.length; i++) { | |
| if (typeof finalizationCallback === 'function') { | |
| finalizationCallback(heldValues[i]); | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.