[steps] Safely index build step inputs and outputs - #4066
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4066 +/- ##
==========================================
+ Coverage 61.54% 61.56% +0.02%
==========================================
Files 980 981 +1
Lines 43952 43957 +5
Branches 9226 9225 -1
==========================================
+ Hits 27047 27056 +9
+ Misses 15457 15453 -4
Partials 1448 1448 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
sswrk
left a comment
There was a problem hiding this comment.
😮
Should we do something in this vein for inputs too?
852b12c to
bf1779d
Compare
|
⏩ The changelog entry check has been skipped since the "no changelog" label is present. |
There was a problem hiding this comment.
Pull request overview
Hardens the @expo/steps runtime against prototype-pollution edge cases by ensuring build-step inputs/outputs and related dictionaries safely support arbitrary identifiers like __proto__, while keeping the public object-shaped interfaces used by step functions.
Changes:
- Introduces a
createEmptyRecord()helper to build null-prototype dictionaries and applies it to input/output indexing and function input evaluation. - Replaces
inchecks withObject.hasOwnwhen reading externally suppliedcallInputsto avoid inherited-property confusion. - Adds targeted regression coverage (including
__proto__) across direct functions, function groups, interpolation,ifconditions, and runtime-loaded custom functions.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/steps/src/utils/record.ts | Adds createEmptyRecord() to create null-prototype dictionaries. |
| packages/steps/src/utils/tests/record-test.ts | Tests the null-prototype record helper. |
| packages/steps/src/scripts/tests/runCustomFunction-test.ts | Expands coverage to ensure __proto__ inputs/outputs work through custom-function execution. |
| packages/steps/src/BuildStepOutput.ts | Uses null-prototype indexing for outputs via createEmptyRecord(). |
| packages/steps/src/BuildStepInput.ts | Uses null-prototype indexing for inputs via createEmptyRecord(). |
| packages/steps/src/BuildStep.ts | Switches ownership checks to Object.hasOwn and ensures evaluated/input dictionaries use null prototypes. |
| packages/steps/src/BuildFunctionGroup.ts | Uses Object.hasOwn to read callInputs safely and updates input indexing helper usage. |
| packages/steps/src/BuildFunction.ts | Uses Object.hasOwn to read callInputs safely. |
| packages/steps/src/tests/StepsConfigParser-test.ts | Updates assertions for empty null-prototype output dictionaries. |
| packages/steps/src/tests/fixtures/my-custom-ts-function/src/index.ts | Extends fixture custom function to use __proto__ inputs/outputs. |
| packages/steps/src/tests/fixtures/my-custom-ts-function/build/index.js | Updates built fixture output for __proto__ handling. |
| packages/steps/src/tests/BuildStepOutput-test.ts | Updates and expands tests for null-prototype output indexing and special keys. |
| packages/steps/src/tests/BuildStepInput-test.ts | Updates and expands tests for null-prototype input indexing and special keys. |
| packages/steps/src/tests/BuildStep-test.ts | Adds regression tests for __proto__ in interpolation, function inputs, deserialization, and if evaluation. |
| packages/steps/src/tests/BuildFunctionGroup-test.ts | Adds coverage for __proto__ call inputs in function groups. |
| packages/steps/src/tests/BuildFunction-test.ts | Adds coverage for __proto__ call inputs in direct build functions. |
| AGENTS.md | Documents the repository convention to use null-prototype records for user-controlled keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
changed it a bit so re-requesting review |
|
thank you for thinking of inputs when reviewing!! |
Why
Build step input and output collections were indexed with ordinary JavaScript objects. An identifier such as
__proto__would interact with the object's prototype instead of behaving like an ordinary own property. Call-input presence checks also treated inherited property names as supplied inputs.How
Object.hasOwnwhen reading externally supplied call inputs.Test Plan
Added regressions for
__proto__identifiers in:ifconditionsValidated locally with the steps and build-tools tests and typechecks, formatting, and lint.