feat(bindings): upstream provenance pins + lock-step review gate for well-known bindings - #7031
Merged
proggeramlug merged 6 commits intoJul 30, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAdds upstream provenance pinning for native bindings, soak validation and autofix changes, CI checks for binding pins and Rust warnings, safer workflow pushes, Skillspector cutoff handling, and iovalkey support in native module dispatch. ChangesRepository controls and native compatibility
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jdalton
added a commit
to jdalton/perry
that referenced
this pull request
Jul 30, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
crates/perry/src/commands/compile/well_known.rs (1)
219-258: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider validating
sha256/dateformat at parse time, matching the lock-step backstop.The code already treats parse-time validation as a defense-in-depth backstop for the lock-step rule ("a skewed pin can't even ship inside the binary" even if
binding_pins.mjs --checkis skipped/buggy). That same reasoning would extend tosha256(should be 64 hex chars) anddate(should beYYYY-MM-DD) — currently only presence/string-ness is checked, so a malformed hash or date would silently ship.♻️ Example format check for sha256
let version = required("version")?; let ported_at = required("ported-at")?; + let sha256 = required("sha256")?; + if sha256.len() != 64 || !sha256.bytes().all(|b| b.is_ascii_hexdigit()) { + return Err(format!( + "[bindings.{}.upstream] sha256 must be a 64-character hex string, got: {}", + pkg_name, sha256 + )); + } if ported_at != version {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry/src/commands/compile/well_known.rs` around lines 219 - 258, Extend the parse-time validation in the upstream pin handling around `required`, `sha256`, and `date` so malformed values are rejected before constructing `UpstreamPin`. Require `sha256` to be exactly 64 hexadecimal characters and `date` to match `YYYY-MM-DD`, while preserving the existing required-field errors and lock-step validation for `ported_at` and `version`.scripts/soak/update-deps.mts (1)
49-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLoop returns on the first present installer, so later candidates are never tried on failure.
paths.mtsdocumentsNPM_INSTALLERSas "tried in order"; only one entry exists today so behavior is fine, but the loop as written can never fall through to a second candidate. Consider falling through on non-zero status (or onres.error) to match the documented intent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/soak/update-deps.mts` around lines 49 - 56, Update the installer loop in the dependency update flow to continue trying later NPM_INSTALLERS entries when run returns a non-zero status or error, and return success immediately when an installer succeeds. Preserve skipping unavailable path-based installers and only emit the “no installer found” message after all candidates fail or are unavailable.scripts/soak/soak.test.mts (1)
32-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the window literals from
SOAK_DAYS/SOAK_MINUTESinstead of hardcoding7/10080.The suite already imports
SOAK_DAYS; hardcoding the same numbers here means a window bump fails these tests for the wrong reason.♻️ Sketch
-import { SOAK_DAYS, addDaysIso, todayIso } from './constants.mts' +import { SOAK_DAYS, SOAK_MINUTES, addDaysIso, todayIso } from './constants.mts' @@ -minimumReleaseAge: 10080 +minimumReleaseAge: ${SOAK_MINUTES} @@ - const good = '[unstable]\nmin-publish-age = true\n\n[registry]\nglobal-min-publish-age = "7 days"\n' + const good = `[unstable]\nmin-publish-age = true\n\n[registry]\nglobal-min-publish-age = "${SOAK_DAYS} days"\n`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/soak/soak.test.mts` around lines 32 - 55, Update the soak tests and CLEAN_YAML fixture to derive expected release windows from the imported SOAK_DAYS and SOAK_MINUTES values instead of hardcoding 7, 10080, or equivalent literals. In the npmrc and cargo assertions, construct matching and mismatching values from those constants while preserving the existing finding and fix behavior..github/workflows/security-audit.yml (1)
67-127: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider an explicit least-privilege
permissions:block on the new jobs.
agent-scanpasses${{ github.token }}to the installer; scoping the job tocontents: read(and the other two jobs likewise) keeps the token minimal regardless of the workflow-level default.#!/bin/bash rg -n 'permissions:' -A4 .github/workflows/security-audit.yml🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/security-audit.yml around lines 67 - 127, Add explicit least-privilege permissions to the new soak-gate, agent-scan, and skills-scan jobs in the workflow, granting only contents: read because agent-scan uses github.token for downloads. Keep the permissions scoped to these jobs and preserve their existing steps.crates/perry-runtime/src/object/global_this/ctor_thunks.rs (1)
47-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer the canonical unboxing helper over an inline payload mask.
(pattern.to_bits() & 0x0000_FFFF_FFFF_FFFF) as usizere-implements pointer extraction;crate::value::js_nanbox_get_pointer(used elsewhere) keeps the mask in one place if the boxing layout ever changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/object/global_this/ctor_thunks.rs` around lines 47 - 52, Update the pointer extraction in the flags_undefined and pattern_value.is_pointer() branch of the surrounding constructor thunk to use crate::value::js_nanbox_get_pointer instead of the inline bitmask, then pass its result to crate::regex::is_regex_pointer while preserving the existing return behavior.crates/perry-runtime/src/object/global_this/populate.rs (1)
107-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComment now mis-describes the arm it precedes. The
#4569: collection constructors throw when called withoutnew`` note sits directly above the new"RegExp"arm, which deliberately does the opposite (it constructs). Move the RegExp arm below the collection arms or give it its own comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/object/global_this/populate.rs` around lines 107 - 110, Correct the misleading comment placement in the global constructor mapping: keep the `#4569` note directly associated with the "Map" and "Set" arms, and move the "RegExp" arm below them or provide it with a separate construction-specific comment.crates/perry/well_known_bindings.toml (1)
242-252: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKey-quoting style is inconsistent between the binding table and its
upstreamsub-table.[bindings."date-fns"]vs[bindings.date-fns.upstream](same forrate-limiter-flexible,node-cron,node-fetch). Both parse to the same table since bare keys permit-, but picking one style avoids readers assuming they're distinct tables.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry/well_known_bindings.toml` around lines 242 - 252, Use consistent quoted table keys for hyphenated binding names in both the parent and upstream tables: update the upstream headers for date-fns, rate-limiter-flexible, node-cron, and node-fetch to match their corresponding quoted binding headers, without changing any binding values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/security-audit.yml:
- Line 22: Update both checkout steps in the security-audit and cargo-deny jobs
to disable credential persistence by adding the same persist-credentials: false
configuration used by the newer jobs.
- Around line 128-137: Update the skillspector reference in the “Scan each
skill” workflow step to use the matching full 40-character commit SHA from
external-tools.json, replacing the abbreviated 2eb84478 ref while preserving the
existing scan command and options.
In `@changelog.d/7021-real-npm-cli-compile-gaps.md`:
- Around line 1-7: Update the changelog heading to state five independent
blockers, matching the five bullets listed in the release notes. Leave the
individual blocker descriptions unchanged.
In `@crates/perry-runtime/src/event_target.rs`:
- Around line 337-358: Update js_dom_exception_subclass_init to create a
RuntimeHandleScope and root message_ptr with root_string_ptr before coercing
name or performing any allocations. Materialize the owned DOMException name
string before subsequent allocation-heavy work, and use the rooted string
handles when setting the message and name properties so GC movement cannot
invalidate pointers.
In `@crates/perry-runtime/src/object/global_this/ctor_thunks.rs`:
- Around line 53-64: Update the RegExp construction flow around js_regexp_new:
create a RuntimeHandleScope, root the coerced pattern and flags values, and
reload both rooted StringHeader pointers after coercion before passing them to
js_regexp_new. Preserve null pointers for undefined inputs and return the
resulting nanboxed regexp pointer as before.
In `@scripts/binding_pins.mjs`:
- Around line 152-173: Add bounded timeouts to the registry requests in
fetchJson and sha256OfTarball, ensuring timeout failures produce actionable
errors. Update the git clone and fetch execFileSync calls in modeMaterialize to
pass an appropriate timeout option, so stalled network operations fail instead
of hanging indefinitely.
In `@scripts/soak/external-tools.mts`:
- Around line 104-115: Update checkPins to require an integrity pin for every
downloadable pin shape, including purl and repository npm pins, rather than only
release: asset pins. Ensure pins lacking integrity are rejected during offline
validation before download() dereferences pin.integrity, while preserving the
existing SHA-512 SRI validation.
- Around line 612-618: Remove the unconditional SFW_UNKNOWN_HOST_ACTION=ignore
export before exec sfw in the shim. Preserve the enterprise default block
behavior, and only set the variable when an explicit opt-in such as
SFW_ALLOW_UNKNOWN_HOSTS=1 is present; otherwise leave it unset.
In `@scripts/soak/soak.mts`:
- Around line 132-146: Update checkNpmrc’s min-release-age regex to accept
optional whitespace around the equals sign, and apply the same tolerant matching
in fixNpmrc so existing spaced key lines are replaced rather than duplicated.
Preserve the current value validation and fixing behavior for missing or
incorrect settings.
---
Nitpick comments:
In @.github/workflows/security-audit.yml:
- Around line 67-127: Add explicit least-privilege permissions to the new
soak-gate, agent-scan, and skills-scan jobs in the workflow, granting only
contents: read because agent-scan uses github.token for downloads. Keep the
permissions scoped to these jobs and preserve their existing steps.
In `@crates/perry-runtime/src/object/global_this/ctor_thunks.rs`:
- Around line 47-52: Update the pointer extraction in the flags_undefined and
pattern_value.is_pointer() branch of the surrounding constructor thunk to use
crate::value::js_nanbox_get_pointer instead of the inline bitmask, then pass its
result to crate::regex::is_regex_pointer while preserving the existing return
behavior.
In `@crates/perry-runtime/src/object/global_this/populate.rs`:
- Around line 107-110: Correct the misleading comment placement in the global
constructor mapping: keep the `#4569` note directly associated with the "Map" and
"Set" arms, and move the "RegExp" arm below them or provide it with a separate
construction-specific comment.
In `@crates/perry/src/commands/compile/well_known.rs`:
- Around line 219-258: Extend the parse-time validation in the upstream pin
handling around `required`, `sha256`, and `date` so malformed values are
rejected before constructing `UpstreamPin`. Require `sha256` to be exactly 64
hexadecimal characters and `date` to match `YYYY-MM-DD`, while preserving the
existing required-field errors and lock-step validation for `ported_at` and
`version`.
In `@crates/perry/well_known_bindings.toml`:
- Around line 242-252: Use consistent quoted table keys for hyphenated binding
names in both the parent and upstream tables: update the upstream headers for
date-fns, rate-limiter-flexible, node-cron, and node-fetch to match their
corresponding quoted binding headers, without changing any binding values.
In `@scripts/soak/soak.test.mts`:
- Around line 32-55: Update the soak tests and CLEAN_YAML fixture to derive
expected release windows from the imported SOAK_DAYS and SOAK_MINUTES values
instead of hardcoding 7, 10080, or equivalent literals. In the npmrc and cargo
assertions, construct matching and mismatching values from those constants while
preserving the existing finding and fix behavior.
In `@scripts/soak/update-deps.mts`:
- Around line 49-56: Update the installer loop in the dependency update flow to
continue trying later NPM_INSTALLERS entries when run returns a non-zero status
or error, and return success immediately when an installer succeeds. Preserve
skipping unavailable path-based installers and only emit the “no installer
found” message after all candidates fail or are unavailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6663a0c8-1faa-4520-97d6-e6772c8d466e
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktools/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
.cargo/config.toml.claude/skills/soak/SKILL.md.github/dependabot.yml.github/workflows/security-audit.yml.github/workflows/soak-autofix.yml.github/workflows/test.yml.github/workflows/zizmor.yml.github/zizmor.yml.npmrcchangelog.d/6912-security-tooling-soak.mdchangelog.d/7021-real-npm-cli-compile-gaps.mdchangelog.d/7031-binding-upstream-lockstep.mdcrates/perry-api-manifest/src/entries.rscrates/perry-codegen/src/expr/this_super_call.rscrates/perry-codegen/src/lower_call/builtin.rscrates/perry-codegen/src/lower_call/native_module_dispatch.rscrates/perry-codegen/src/lower_call/new_helpers.rscrates/perry-codegen/src/runtime_decls/strings_part2.rscrates/perry-ext-zlib/Cargo.tomlcrates/perry-ext-zlib/src/stream.rscrates/perry-runtime/src/event_target.rscrates/perry-runtime/src/object/global_this.rscrates/perry-runtime/src/object/global_this/ctor_thunks.rscrates/perry-runtime/src/object/global_this/populate.rscrates/perry/src/commands/compile/optimized_libs.rscrates/perry/src/commands/compile/optimized_libs/driver.rscrates/perry/src/commands/compile/optimized_libs/freshness.rscrates/perry/src/commands/compile/optimized_libs/tests.rscrates/perry/src/commands/compile/strip_dedup.rscrates/perry/src/commands/compile/well_known.rscrates/perry/src/commands/stdlib_features.rscrates/perry/well_known_bindings.tomldocs/src/SUMMARY.mddocs/src/native-libraries/upstream-pins.mdexternal-tools.jsonpackage.jsonscripts/binding_pins.mjsscripts/soak/constants.mtsscripts/soak/external-tools.mtsscripts/soak/external-tools.test.mtsscripts/soak/paths.mtsscripts/soak/soak.mtsscripts/soak/soak.test.mtsscripts/soak/update-deps.mtsscripts/soak/update-deps.test.mtstools/package.jsontools/pnpm-workspace.yamltools/taze.config.mts
Comment on lines
+337
to
+358
| pub extern "C" fn js_dom_exception_subclass_init(this_value: f64, message: f64, name: f64) -> f64 { | ||
| let Some(exception) = value_as_ptr::<ObjectHeader>(this_value) else { | ||
| return undefined_value(); | ||
| }; | ||
| let message_ptr = optional_string_from_value(message, b""); | ||
| let name_ptr = optional_string_from_value(name, b"Error"); | ||
| let name_string = unsafe { | ||
| let len = (*name_ptr).byte_len as usize; | ||
| let data = (name_ptr as *const u8).add(std::mem::size_of::<StringHeader>()); | ||
| String::from_utf8_lossy(std::slice::from_raw_parts(data, len)).into_owned() | ||
| }; | ||
| set_event_field( | ||
| exception, | ||
| b"message", | ||
| crate::value::js_nanbox_string(message_ptr as i64), | ||
| ); | ||
| set_event_field( | ||
| exception, | ||
| b"name", | ||
| crate::value::js_nanbox_string(name_ptr as i64), | ||
| ); | ||
| set_event_field(exception, b"code", dom_exception_code(&name_string)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^crates/perry-runtime/src/|README|gc|handle|RuntimeHandleScope|GC|handle_root)' || true
echo "== event_target relevant section =="
wc -l crates/perry-runtime/src/event_target.rs
sed -n '300,380p' crates/perry-runtime/src/event_target.rs
echo "== optional_string / value_as_ptr / gc roots definitions =="
rg -n "fn optional_string_from_value|fn string_from_value|value_as_ptr|RuntimeHandleScope|register.*root|gc|mark|move|conservativ" crates/perry-runtime/src -S || trueRepository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== precise event_target section =="
sed -n '320,365p' crates/perry-runtime/src/value.rs || true
sed -n '320,365p' crates/perry-runtime/src/arena.rs || true
rg -n "pub fn optional_string_from_value|fn string_from_value|pub fn value_as_ptr|pub struct RuntimeHandleScope|struct RuntimeHandle" crates/perry-runtime/src -S --max-count 20 || true
echo "== string_from_value definitions =="
rg -n "pub fn string_from_value|fn string_from_value|fn js_coerce_to_string|fn maybe_to_string" crates/perry-runtime/src/value.rs crates/perry-runtime/src -S --max-count 40 || true
echo "== gc allocator allocation point =="
rg -n "fn arena_alloc_gc|fn gc_malloc|fn gc_alloc|RuntimeHandleScope::new|scan_rust_locals|conservative" crates/perry-runtime/src/gc.rs crates/perry-runtime/src/arena.rs crates/perry-runtime/src/lib.rs -S --max-count 80 || trueRepository: PerryTS/perry
Length of output: 2156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== precise helper definitions =="
rg -n "pub fn optional_string_from_value|fn string_from_value|pub fn value_as_ptr|struct RuntimeHandleScope|impl RuntimeHandleScope|fn RuntimeHandleScope::new" crates/perry-runtime/src -S || true
echo "== event_target relevant section =="
sed -n '320,365p' crates/perry-runtime/src/event_target.rs || true
echo "== relevant value/string coercion definitions =="
rg -n "pub fn string_from_value|fn string_from_value|fn js_coerce_to_string|fn maybe_to_string" crates/perry-runtime/src -S || true
echo "== GC root/scan and allocation relevant definitions =="
rg -n "scan_rust_locals|conservativ|alloc.*gc|fn arena_alloc_gc|fn gc_malloc|RuntimeHandleScope::new" crates/perry-runtime/src/gc.rs crates/perry-runtime/src/arena.rs crates/perry-runtime/src/lib.rs -S || trueRepository: PerryTS/perry
Length of output: 3908
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== event_target top section =="
sed -n '1,110p' crates/perry-runtime/src/event_target.rs
echo "== event_target later section =="
sed -n '355,395p' crates/perry-runtime/src/event_target.rs
echo "== candidate files with value_as_ptr/optional_string_from_value =="
rg -n "fn value_as_ptr|fn optional_string_from_value|fn set_event_field|struct ObjectHeader|struct StringHeader" crates/perry-runtime/src -S || true
echo "== gc runtime_handles sections =="
sed -n '1,220p' crates/perry-runtime/src/gc/roots/runtime_handles.rs
echo "== GC allocation/root registry references =="
rg -n "fn arena_alloc_gc|fn gc_malloc|scan_rust_locals|conservativ|register.*root|register.*gc|RuntimeHandleScope::new" crates/perry-runtime/src/gc crates/perry-runtime/src/gc/roots crates/perry-runtime/src -S --max-count 120 || true
echo "== list files to pin layout =="
git ls-files crates/perry-runtime/src | head -80Repository: PerryTS/perry
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -u
echo "== js_string_coerce definition =="
rg -n "pub fn js_string_coerce|fn js_string_coerce" crates/perry-runtime/src -S || true
sed -n '355,395p' crates/perry-runtime/src/event_target.rs
echo "== key helper definitions around string_from_value =="
sed -n '80,92p' crates/perry-runtime/src/event_target.rs
echo "== RuntimeHandleScope raw pointer/root API =="
rg -n "pub fn root_string_ptr|pub fn root_raw_const_ptr|set_raw_mut_ptr|refreshed" crates/perry-runtime/src/gc/roots/runtime_handles.rs
echo "== focused GC allocator trigger references =="
rg -n "pub fn gc_check_trigger|fn gc_check_trigger|pub fn gc|gc_check" crates/perry-runtime/src -S --max-count 120 || trueRepository: PerryTS/perry
Length of output: 12895
🏁 Script executed:
#!/bin/bash
set -u
echo "== js_string_coerce =="
sed -n '600,645p' crates/perry-runtime/src/builtins/numbers.rs
echo "== RuntimeHandleScope root_string_ptr =="
sed -n '80,98p' crates/perry-runtime/src/gc/roots/runtime_handles.rs
sed -n '200,230p' crates/perry-runtime/src/gc/roots/runtime_handles.rs
echo "== gc_check_trigger allocation safety comments =="
sed -n '1100,1130p' crates/perry-runtime/src/gc/policy.rs
sed -n '218,240p' crates/perry-runtime/src/gc/malloc.rsRepository: PerryTS/perry
Length of output: 5916
Root the GC-managed strings before further allocation.
exception isn’t a GC-managed JS heap object here, but message_ptr can be a heap string returned directly from js_string_coerce, and it is kept across the allocation-heavy js_string_coerce(name), from_utf8_lossy().into_owned(), and property sets. Root it with a RuntimeHandleScope/root_string_ptr handle and materialize the owned DOMException name string before string-coercing name, then property-set by handle so GC-completed pointers are still valid.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/event_target.rs` around lines 337 - 358, Update
js_dom_exception_subclass_init to create a RuntimeHandleScope and root
message_ptr with root_string_ptr before coercing name or performing any
allocations. Materialize the owned DOMException name string before subsequent
allocation-heavy work, and use the rooted string handles when setting the
message and name properties so GC movement cannot invalidate pointers.
Source: Learnings
Comment on lines
+53
to
+64
| let pattern_ptr = if pattern_value.is_undefined() { | ||
| std::ptr::null_mut() | ||
| } else { | ||
| crate::builtins::js_string_coerce(pattern) | ||
| }; | ||
| let flags_ptr = if flags_undefined { | ||
| std::ptr::null_mut() | ||
| } else { | ||
| crate::builtins::js_string_coerce(flags) | ||
| }; | ||
| let re = crate::regex::js_regexp_new(pattern_ptr, flags_ptr); | ||
| crate::value::js_nanbox_pointer(re as i64) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep run --lang rust --pattern 'pub extern "C" fn js_string_coerce($$$) { $$$ }' crates/perry-runtime/src
rg -n -C5 'fn js_regexp_new' crates/perry-runtime/src/regex
rg -n -C6 'RuntimeHandleScope' crates/perry-runtime/src/object/class_registry/construct.rsRepository: PerryTS/perry
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -u
echo "== file listing relevant to runtime =="
git ls-files 'crates/perry-runtime/src/**/*.rs' | sed -n '1,160p'
echo
echo "== locate ctor_thunks.rs and nearby content =="
fd -a 'ctor_thunks\.rs$' . || true
file="$(fd 'ctor_thunks\.rs$' . | head -n1 || true)"
if [ -n "${file:-}" ]; then
wc -l "$file"
cat -n "$file"
fi
echo
echo "== locate js_string_coerce definitions/usages =="
rg -n -C4 'js_string_coerce|fn str_to_js_string|to.*string' crates/perry-runtime/src || true
echo
echo "== locate js_regexp_new definition/usages =="
rg -n -C4 'js_regexp_new|fn js_regexp_new|regex_new|reg.new' crates/perry-runtime/src || true
echo
echo "== locate RuntimeHandleScope definitions/usages =="
rg -n -C4 'RuntimeHandleScope|RuntimeHandle' crates/perry-runtime/src || trueRepository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
echo "== exact file location =="
fd '^ctor_thunks\.rs$' .
file="$(fd '^ctor_thunks\.rs$' . | head -n1)"
if [ -n "${file:-}" ]; then
echo
echo "== lines 1-140 of $file =="
cat -n "$file" | sed -n '1,140p'
fi
echo
echo "== js_string_coerce implementation/usages =="
rg -n "js_string_coerce|fn str_to_js_string|StringHeader|to_string" crates/perry-runtime/src -g '*.rs' --glob '!**/builtins.rs' --max-count 120 || true
echo
echo "== js_regexp_new definition/usages =="
rg -n "js_regexp_new|fn .*regexp|RegexpHeader|new RegExp" crates/perry-runtime/src -g '*.rs' --max-count 80 || true
echo
echo "== RuntimeHandleScope API =="
rg -n "pub struct RuntimeHandleScope|impl RuntimeHandleScope|enum RuntimeHandle|root_nanbox_f64|root_raw_mut_ptr|root_raw_const_ptr|RootSourceSlotTraceStats" crates/perry-runtime/src/gc crates/perry-runtime/src -g '*.rs' --max-count 160 || trueRepository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
file="$(fd '^ctor_thunks\.rs$' . | head -n1)"
echo "file=$file"
if [ -n "${file:-}" ]; then
wc -l "$file"
cat -n "$file"
fi
echo
echo "== RegexBuilder/regexp implementations files =="
fd '.*(regex|regexp).*\.rs$|builtins.*\.rs$|gc.*\.rs$|string.*\.rs$|value.*\.rs$' crates/perry-runtime/src | sort
echo
echo "== focused definitions, excluding massive match trees where possible =="
rg -n "pub fn js_string_coerce|fn js_regexp_new|struct RegexBuilder|enum RegExp|root_nanbox_f64|root_raw_mut_ptr|root_raw_const_ptr|alloc\\(" crates/perry-runtime/src/gc crates/perry-runtime/src/regex crates/perry-runtime/src/builtins crates/perry-runtime/src/value crates/perry-runtime/src/string -g '*.rs' --max-count 200 || trueRepository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -u
echo "== regex.rs core exports =="
wc -l crates/perry-runtime/src/regex.rs
cat -n crates/perry-runtime/src/regex.rs | sed -n '1,240p'
echo
echo "== builtins relevant string coercion =="
wc -l crates/perry-runtime/src/builtins/globals.rs
cat -n crates/perry-runtime/src/builtins/globals.rs | sed -n '1,140p'
rg -n -C8 "pub.*fn js_string_coerce|fn js_string_coerce|pub fn js_\\*to_string|fn js_\\*to_string|ToPrimitive|to_string" crates/perry-runtime/src/builtins crates/perry-runtime/src/value/to_string.rs crates/perry-runtime/src/value/jsvalue.rs -g '*.rs' --max-count 160 || true
echo
echo "== RuntimeHandleScope root API =="
cat -n crates/perry-runtime/src/gc/roots/runtime_handles.rs | sed -n '1,140p'Repository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
file="$(fd '^ctor_thunks\.rs$' . | head -n1)"
echo "file=$file"
wc -l "$file"
cat -n "$file" | sed -n '1,140p'
echo
echo "== regex.rs relevant exports =="
wc -l crates/perry-runtime/src/regex.rs
cat -n crates/perry-runtime/src/regex.rs | sed -n '1,240p'
echo
echo "== RegExp construction references in perry-runtime =="
rg -n "js_regexp_new|RegExp|regexp_construct|is_regex_pointer" crates/perry-runtime/src -g '*.rs' --max-count 160 || true
echo
echo "== RuntimeHandleScope/Handle API relevant =="
cat -n crates/perry-runtime/src/gc/roots/runtime_handles.rs | sed -n '1,120p'Repository: PerryTS/perry
Length of output: 50369
Root pattern before coercing flags. js_string_coerce(flags) can allocate/run user code and GC-evacuate the pattern StringHeader behind pattern_ptr before js_regexp_new reads both raw strings. Wrap the construction in a RuntimeHandleScope, root the coerced inputs, and reload them before calling js_regexp_new.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/object/global_this/ctor_thunks.rs` around lines 53 -
64, Update the RegExp construction flow around js_regexp_new: create a
RuntimeHandleScope, root the coerced pattern and flags values, and reload both
rooted StringHeader pointers after coercion before passing them to
js_regexp_new. Preserve null pointers for undefined inputs and return the
resulting nanboxed regexp pointer as before.
Source: Learnings
Comment on lines
+152
to
+173
| async function fetchJson(url) { | ||
| const res = await fetch(url, { headers: { accept: 'application/json' } }); | ||
| if (!res.ok) throw new Error(`${url}: HTTP ${res.status}`); | ||
| return res.json(); | ||
| } | ||
|
|
||
| async function fetchPackument(name) { | ||
| return fetchJson(`${REGISTRY}/${name.replace('/', '%2f')}`); | ||
| } | ||
|
|
||
| function latestStable(packument) { | ||
| const version = packument['dist-tags']?.latest; | ||
| if (!version) throw new Error(`${packument.name}: no dist-tags.latest`); | ||
| return version; | ||
| } | ||
|
|
||
| async function sha256OfTarball(url) { | ||
| const res = await fetch(url); | ||
| if (!res.ok) throw new Error(`${url}: HTTP ${res.status}`); | ||
| const bytes = Buffer.from(await res.arrayBuffer()); | ||
| return createHash('sha256').update(bytes).digest('hex'); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
No timeouts on registry fetches or git subprocess calls.
fetchJson/sha256OfTarball (used by --set/--backfill/--check --refresh) and the git clone/git fetch calls in modeMaterialize have no timeout. --check --refresh is documented as the weekly CI job (Line 31-34, 77-81 of the header comment) — a registry stall or an unresponsive git remote will hang that job indefinitely instead of failing fast with an actionable error.
🕒 Proposed fix: add fetch timeouts
async function fetchJson(url) {
- const res = await fetch(url, { headers: { accept: 'application/json' } });
+ const res = await fetch(url, {
+ headers: { accept: 'application/json' },
+ signal: AbortSignal.timeout(30_000),
+ });
if (!res.ok) throw new Error(`${url}: HTTP ${res.status}`);
return res.json();
} async function sha256OfTarball(url) {
- const res = await fetch(url);
+ const res = await fetch(url, { signal: AbortSignal.timeout(60_000) });
if (!res.ok) throw new Error(`${url}: HTTP ${res.status}`);For the execFileSync('git', ...) calls, consider passing a timeout option as well.
Also applies to: 309-338
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/binding_pins.mjs` around lines 152 - 173, Add bounded timeouts to the
registry requests in fetchJson and sha256OfTarball, ensuring timeout failures
produce actionable errors. Update the git clone and fetch execFileSync calls in
modeMaterialize to pass an appropriate timeout option, so stalled network
operations fail instead of hanging indefinitely.
Comment on lines
+104
to
+115
| const integrities = [ | ||
| ...(pin.integrity ? [pin.integrity] : []), | ||
| ...Object.values(pin.platforms ?? {}).map(p => p.integrity), | ||
| ] | ||
| if (pin.release === 'asset' && integrities.length === 0) { | ||
| out.push(`${name}: release asset without any integrity pin`) | ||
| } | ||
| for (const sri of integrities) { | ||
| if (!/^sha512-[A-Za-z0-9+/]+={0,2}$/.test(sri)) { | ||
| out.push(`${name}: integrity is not a sha512 SRI: ${sri}`) | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Integrity is only required for release: asset pins, yet purl/npm-tarball installs dereference pin.integrity!.
A purl or repository: npm: pin with no integrity passes --check and then fails at install time inside download() as a confusing "integrity mismatch ... expected undefined". Require an integrity pin for every downloadable shape in checkPins so the gate catches it offline.
🛡 Proposed check
- if (pin.release === 'asset' && integrities.length === 0) {
- out.push(`${name}: release asset without any integrity pin`)
+ const downloadable =
+ pin.release === 'asset' || Boolean(pin.purl) || pin.repository?.startsWith('npm:')
+ if (downloadable && integrities.length === 0) {
+ out.push(`${name}: downloadable pin without any integrity pin`)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const integrities = [ | |
| ...(pin.integrity ? [pin.integrity] : []), | |
| ...Object.values(pin.platforms ?? {}).map(p => p.integrity), | |
| ] | |
| if (pin.release === 'asset' && integrities.length === 0) { | |
| out.push(`${name}: release asset without any integrity pin`) | |
| } | |
| for (const sri of integrities) { | |
| if (!/^sha512-[A-Za-z0-9+/]+={0,2}$/.test(sri)) { | |
| out.push(`${name}: integrity is not a sha512 SRI: ${sri}`) | |
| } | |
| } | |
| const integrities = [ | |
| ...(pin.integrity ? [pin.integrity] : []), | |
| ...Object.values(pin.platforms ?? {}).map(p => p.integrity), | |
| ] | |
| const downloadable = | |
| pin.release === 'asset' || Boolean(pin.purl) || pin.repository?.startsWith('npm:') | |
| if (downloadable && integrities.length === 0) { | |
| out.push(`${name}: downloadable pin without any integrity pin`) | |
| } | |
| for (const sri of integrities) { | |
| if (!/^sha512-[A-Za-z0-9+/]+={0,2}$/.test(sri)) { | |
| out.push(`${name}: integrity is not a sha512 SRI: ${sri}`) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/soak/external-tools.mts` around lines 104 - 115, Update checkPins to
require an integrity pin for every downloadable pin shape, including purl and
repository npm pins, rather than only release: asset pins. Ensure pins lacking
integrity are rejected during offline validation before download() dereferences
pin.integrity, while preserving the existing SHA-512 SRI validation.
Comment on lines
+612
to
+618
| # Enterprise sfw defaults to BLOCK for non-registry hosts | ||
| # (SFW_UNKNOWN_HOST_ACTION, parsed by the enterprise config), which | ||
| # breaks ordinary dev flows the day a Socket key lands. Only the | ||
| # enterprise build reads the var — it is inert for the free tier — so | ||
| # setting it unconditionally is safe. | ||
| export SFW_UNKNOWN_HOST_ACTION=ignore | ||
| exec sfw '${cmd}' "$@" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Unconditionally exporting SFW_UNKNOWN_HOST_ACTION=ignore disables the firewall's block-by-default posture.
Every shimmed package manager (npm/yarn/pnpm/pip/uv/cargo) will run with unknown-host blocking off the moment a Socket key lands, which is the main protection the shims exist to provide. Prefer leaving the enterprise default in place and letting an operator opt out per-invocation (e.g. only set it when an explicit env like SFW_ALLOW_UNKNOWN_HOSTS=1 is present), or default to a warning-level action rather than ignore.
🔒 Opt-in instead of always-off
-export SFW_UNKNOWN_HOST_ACTION=ignore
+# Opt-in escape hatch: unknown-host blocking stays ON by default.
+if [ -n "\${SFW_ALLOW_UNKNOWN_HOSTS:-}" ]; then
+ export SFW_UNKNOWN_HOST_ACTION=ignore
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Enterprise sfw defaults to BLOCK for non-registry hosts | |
| # (SFW_UNKNOWN_HOST_ACTION, parsed by the enterprise config), which | |
| # breaks ordinary dev flows the day a Socket key lands. Only the | |
| # enterprise build reads the var — it is inert for the free tier — so | |
| # setting it unconditionally is safe. | |
| export SFW_UNKNOWN_HOST_ACTION=ignore | |
| exec sfw '${cmd}' "$@" | |
| # Enterprise sfw defaults to BLOCK for non-registry hosts | |
| # (SFW_UNKNOWN_HOST_ACTION, parsed by the enterprise config), which | |
| # breaks ordinary dev flows the day a Socket key lands. Only the | |
| # enterprise build reads the var — it is inert for the free tier — so | |
| # setting it unconditionally is safe. | |
| # Opt-in escape hatch: unknown-host blocking stays ON by default. | |
| if [ -n "\${SFW_ALLOW_UNKNOWN_HOSTS:-}" ]; then | |
| export SFW_UNKNOWN_HOST_ACTION=ignore | |
| fi | |
| exec sfw '${cmd}' "$@" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/soak/external-tools.mts` around lines 612 - 618, Remove the
unconditional SFW_UNKNOWN_HOST_ACTION=ignore export before exec sfw in the shim.
Preserve the enterprise default block behavior, and only set the variable when
an explicit opt-in such as SFW_ALLOW_UNKNOWN_HOSTS=1 is present; otherwise leave
it unset.
Comment on lines
+132
to
+146
| export function checkNpmrc(body: string, file: string): Finding[] { | ||
| const out: Finding[] = [] | ||
| const days = /^min-release-age=(\d+)\s*$/m.exec(body)?.[1] | ||
| if (Number(days) !== SOAK_DAYS) { | ||
| out.push({ | ||
| file, | ||
| what: 'npm min-release-age window', | ||
| saw: days ?? '(missing)', | ||
| wanted: String(SOAK_DAYS), | ||
| fix: `set min-release-age=${SOAK_DAYS} (or run --fix)`, | ||
| }) | ||
| } | ||
| out.push(...checkNpmrcExcludes(body, file)) | ||
| return out | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
min-release-age is only recognized without spaces around =.
npm's ini parser accepts min-release-age = 7; that form is reported as (missing) here and fixNpmrc then appends a second key line (last one wins in ini, so the file ends up with a duplicate). Tolerating optional whitespace in both the check and the fixer avoids the false finding.
🛠 Proposed tolerance for spaced keys
- const days = /^min-release-age=(\d+)\s*$/m.exec(body)?.[1]
+ const days = /^min-release-age[ \t]*=[ \t]*(\d+)[ \t]*$/m.exec(body)?.[1]And in fixNpmrc (lines 504-505):
- if (/^min-release-age=\d+[ \t]*$/m.test(body)) {
- return body.replace(/^min-release-age=\d+[ \t]*$/m, `min-release-age=${SOAK_DAYS}`)
+ const key = /^min-release-age[ \t]*=[ \t]*\d+[ \t]*$/m
+ if (key.test(body)) {
+ return body.replace(key, `min-release-age=${SOAK_DAYS}`)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/soak/soak.mts` around lines 132 - 146, Update checkNpmrc’s
min-release-age regex to accept optional whitespace around the equals sign, and
apply the same tolerant matching in fixNpmrc so existing spaced key lines are
replaced rather than duplicated. Preserve the current value validation and
fixing behavior for missing or incorrect settings.
…own bindings Port the socket-registry fleet's upstream-reference technique to well_known_bindings.toml: every third-party npm package with a bundled native wrapper records which upstream release it ports (version, tarball sha256, repo, git ref) and when it was last reviewed against it (ported-at, date). The lock-step rule — ported-at == version — means a pin bump can't outrun the wrapper review it demands. Enforced twice: the toml parser inside perry refuses to load a skewed table, and scripts/binding_pins.mjs --check gates it in CI (offline). Node builtins (node-builtin=true), aliases (alias-of), and perry-owned packages are exempt from pinning; a distinct package sharing a wrapper crate (redis, iovalkey) still carries its own pin. scripts/binding_pins.mjs provisions pins from the npm registry (--set/--backfill), gates them offline (--check), flags newly-soaked upstream releases (--check --refresh --soak-days N), and materializes an upstream at its pin for port review (--materialize). All 34 third-party bindings backfilled at their current latest. Also adds an iovalkey binding — the Valkey fork of ioredis (valkey-io/iovalkey), a distinct npm package served by the existing perry-ext-ioredis surface — wired through NATIVE_MODULES, the codegen module-normalization + Redis-constructor lists, and stdlib feature gating, and annotates every NATIVE_MODULES entry. 758 tests pass; binding_pins.mjs --check green.
proggeramlug
force-pushed
the
feat/binding-upstream-lockstep
branch
from
July 30, 2026 06:37
b9aca32 to
5462879
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/test.yml (1)
791-799: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
gc-stresscheckout is missingpersist-credentials: false, unlikewindows-buildin the same file.This job now runs unconditionally on every
pull_request/push/workflow_dispatch(previously label-gated) and builds PR-controlled code (cargo build --release -p perry -p perry-runtime -p perry-stdlib ...), the same risk categorywindows-build(lines 737-742) explicitly guards against withpersist-credentials: false("compiles PR-controlled build scripts / proc macros; don't leave the workflow token in .git/config for them to read"). Widening the trigger here materially increases exposure of this gap.🔒 Proposed fix
steps: - uses: actions/checkout@v7 + with: + persist-credentials: false - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml around lines 791 - 799, Update the checkout step in the gc-stress job to disable credential persistence by setting persist-credentials to false, matching the existing windows-build checkout configuration. Keep the job triggers and remaining checkout options unchanged.
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
791-797: 🧹 Nitpick | 🔵 Trivial
gc-stressnow runs unconditionally on every PR with a 90-min timeout.Trigger is no longer label-gated and has no path filter, so even doc-only/non-Rust PRs pay the full release-build + GC matrix cost. The rationale (cost-split PR-subset vs full arms) is well documented in the surrounding comments, so this is just a note for awareness rather than a defect — consider a path filter if CI cost/latency becomes a concern once this job is added to required branch-protection contexts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml around lines 791 - 797, Review the gc-stress workflow trigger and add an appropriate path filter if CI cost becomes a concern, limiting execution to relevant Rust or build-related changes while preserving workflow_dispatch support and required branch-protection behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/test.yml:
- Around line 791-799: Update the checkout step in the gc-stress job to disable
credential persistence by setting persist-credentials to false, matching the
existing windows-build checkout configuration. Keep the job triggers and
remaining checkout options unchanged.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 791-797: Review the gc-stress workflow trigger and add an
appropriate path filter if CI cost becomes a concern, limiting execution to
relevant Rust or build-related changes while preserving workflow_dispatch
support and required branch-protection behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4baec057-c215-4e48-8d17-66789e150f7f
📒 Files selected for processing (6)
.github/workflows/security-audit.yml.github/workflows/test.ymlchangelog.d/7031-binding-upstream-lockstep.mdcrates/perry-api-manifest/src/entries.rscrates/perry-codegen/src/lower_call/builtin.rscrates/perry-codegen/src/lower_call/native_module_dispatch.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/perry-api-manifest/src/entries.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/test.yml (1)
280-280: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPin checkout and disable credential persistence in the rustc-warnings Cargo job.
actions/checkout@v7is a movable tag and defaultspersist-credentialsto true; this job then runscargo check, which can invoke build scripts during compilation. Pin the action to its commit SHA and addpersist-credentials: false, matching the security-audit workflows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml at line 280, Update the checkout step in the rustc-warnings Cargo job to pin actions/checkout to the approved immutable commit SHA and set persist-credentials to false. Keep the job’s existing checkout behavior unchanged aside from these security settings, matching the configuration used by the security-audit workflows.
♻️ Duplicate comments (1)
scripts/soak/external-tools.mts (1)
108-111: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winValidate actual integrity values, not platform-entry count.
integrities.lengthcountsundefined, so a downloadable pin with a platform entry but no hash passes--check. Require a non-empty integrity for each downloadable resolution path; otherwise installation reaches the missing integrity later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/soak/external-tools.mts` around lines 108 - 111, Update the downloadable-pin validation around downloadable and integrities so it checks for at least one non-empty integrity value rather than relying on integrities.length. Ensure entries containing only undefined or empty hashes still report “downloadable pin without any integrity pin,” while valid integrity values pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/soak/soak.mts`:
- Around line 432-445: Extend the fixer containing the registrySection and key
logic to also create or update the [unstable] section with min-publish-age =
true when it is missing or incorrect. Ensure --fix applies both configuration
changes before validation, and add a regression assertion confirming the fixed
output produces zero Cargo findings.
---
Outside diff comments:
In @.github/workflows/test.yml:
- Line 280: Update the checkout step in the rustc-warnings Cargo job to pin
actions/checkout to the approved immutable commit SHA and set
persist-credentials to false. Keep the job’s existing checkout behavior
unchanged aside from these security settings, matching the configuration used by
the security-audit workflows.
---
Duplicate comments:
In `@scripts/soak/external-tools.mts`:
- Around line 108-111: Update the downloadable-pin validation around
downloadable and integrities so it checks for at least one non-empty integrity
value rather than relying on integrities.length. Ensure entries containing only
undefined or empty hashes still report “downloadable pin without any integrity
pin,” while valid integrity values pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: beaceb25-5483-423f-acdc-4b411b1fd634
📒 Files selected for processing (10)
.github/workflows/security-audit.yml.github/workflows/soak-autofix.yml.github/workflows/test.ymlchangelog.d/6912-security-tooling-soak.mdexternal-tools.jsonscripts/soak/constants.mtsscripts/soak/external-tools.mtsscripts/soak/external-tools.test.mtsscripts/soak/soak.mtsscripts/soak/soak.test.mts
Comment on lines
+432
to
+445
| const registrySection = /^\[registry\][^[]*/ms | ||
| const key = /^(global-min-publish-age\s*=\s*)"[^"]*"/m | ||
| if (registrySection.test(body)) { | ||
| return body.replace(registrySection, section => { | ||
| if (key.test(section)) { | ||
| return section.replace(key, `$1"${SOAK_DAYS} days"`) | ||
| } | ||
| return section.replace( | ||
| /^\[registry\][ \t]*$/m, | ||
| `[registry]\nglobal-min-publish-age = "${SOAK_DAYS} days"`, | ||
| ) | ||
| }) | ||
| } | ||
| return `${body.trimEnd()}\n\n[registry]\nglobal-min-publish-age = "${SOAK_DAYS} days"\n` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make --fix add the required unstable feature gate too.
For input without [unstable] min-publish-age = true, this fixer adds [registry] but the subsequent check still reports a finding and exits nonzero. Extend the fixer to create or repair the [unstable] block as well, and add a regression assertion that the fixed result has zero Cargo findings.
As per PR objectives, “adds missing configuration keys during autofix.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/soak/soak.mts` around lines 432 - 445, Extend the fixer containing
the registrySection and key logic to also create or update the [unstable]
section with min-publish-age = true when it is missing or incorrect. Ensure
--fix applies both configuration changes before validation, and add a regression
assertion confirming the fixed output produces zero Cargo findings.
proggeramlug
pushed a commit
to jdalton/perry
that referenced
this pull request
Jul 30, 2026
proggeramlug
pushed a commit
to jdalton/perry
that referenced
this pull request
Jul 30, 2026
…ork end-to-end
The perry-ext-node-forge crypto core (added earlier this branch) was
inert from compiled TypeScript: forge's API is deeply nested
(forge.pki.rsa.generateKeyPair, forge.pki.createCertificate,
forge.md.sha256.create), so the call receiver is a Member CHAIN, not a
bare native-module Ident — no dispatch arm fired, and an intermediate
read like forge.pki hit the unimplemented-API gate (no node-forge
symbol 'pki') and deferred a throw.
Adds try_node_forge_namespace in perry-hir's call lowering: collapse any
member chain rooted at the node-forge default import to its last segment
(the method key the codegen NATIVE_MODULE_TABLE rows already use). Runs
before the generic namespace / module.Class.staticMethod dispatch, which
would otherwise claim the 2-level forge.pki.createCertificate() shape and
gate its forge.pki object-read.
Also fixes a wrapper fidelity gap: setSubject/setIssuer now store
node-forge's { attributes: [...] } DN shape (matching certificateFromPem),
so leaf.setIssuer(caCert.subject.attributes) — the sfw idiom — reads the
CA subject back and the leaf's issuer DN matches the CA. certificateToPem
accepts both the wrapped and bare-array shapes.
End-to-end: a CA+leaf generated by the COMPILED binary from real
forge.pki.* TypeScript (fixture with no node-forge installed, served by
the well-known table) now verifies with the openssl CLI
(leaf.pem: OK; issuer=CN=Socket Security CA), correct SANs / keyUsage /
extKeyUsage / sha256WithRSAEncryption. Adds an upstream provenance pin
(lock-step per PerryTS#7031) and 3 HIR flattening tests.
perry bin 753 pass; node-forge crate 6+1 (incl. openssl e2e); new HIR
tests 3/3. (3 pre-existing write-PIC codegen failures on this worktree
are unrelated — they fail identically on branches with none of this
work.)
proggeramlug
pushed a commit
that referenced
this pull request
Jul 30, 2026
…#7032) * feat(ext): add perry-ext-undici — undici served by the native fetch stack (#466) Bare `import { ProxyAgent, setGlobalDispatcher, fetch, Agent, getGlobalDispatcher } from 'undici'` now resolves through the well-known bindings table to a thin native wrapper instead of AOT-compiling undici's ~50k lines of JS. - crates/perry-ext-undici: ProxyAgent(uri | {uri, token?}) / Agent dispatcher handles; setGlobalDispatcher pushes the proxy config into the shared fetch client via the new js_fetch_set_global_proxy symbol; getGlobalDispatcher returns the installed handle (lazily creating the implicit global Agent); request() rejects with a clear not-implemented error pointing at fetch. - perry-stdlib fetch + perry-ext-fetch (mirror): global proxy override state + proxied reqwest client (CONNECT tunneling for https, absolute-form for http, token sent as Proxy-Authorization); every fetch path now routes through fetch_client(). - perry-hir: bare-ident `new ProxyAgent(...)` / `new Agent(...)` imported from undici lowers to receiver-less NativeMethodCall (mirrors the http/https Agent arm), so locals tag as (undici, ProxyAgent|Agent) instances for close()/destroy() dispatch. - perry-codegen: UNDICI_ROWS dispatch family (constructors, set/getGlobalDispatcher, request, close/destroy). `fetch` from undici reuses the name-keyed Expr::FetchWithOptions lowering. - perry CLI: well_known_bindings.toml row, shared-tokio set (+ codegen-units pins), web-fetch re-assert in the flip loop, sandbox-profile network unlock, PERRY_NATIVE_EXTENSION_PACKAGES guard, manifest NATIVE_MODULES + API entries, regenerated API docs. Targets undici's stable dispatcher API (unchanged across 6.x -> 7.x). E2E-verified against a local mock proxy: proxied fetch carries the ProxyAgent token as Proxy-Authorization, installing a plain Agent restores direct connections. * feat(bindings): pin undici upstream provenance (lock-step, PR #7031) * docs: key changelog fragment to PR #7032 * fix(undici): preserve proxy state and require native routing --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
pushed a commit
that referenced
this pull request
Jul 30, 2026
…to-end from TypeScript (#7033) * feat(ext): add native node-forge PKI binding for Socket Firewall's TLS-MITM CA Add `perry-ext-node-forge`, a native wrapper for the exact node-forge PKI subset sfw's TLS-MITM CA uses, so apps stop AOT-compiling forge's pure-JS bignum/ASN.1/crypto code: - forge.pki.rsa.generateKeyPair({ bits }) -> native RSA keygen - forge.pki.createCertificate() builder: settable publicKey / serialNumber / validity.notBefore|notAfter, setSubject / setIssuer / setExtensions (basicConstraints, keyUsage, extKeyUsage, subjectAltName, subjectKeyIdentifier), sign(privateKey, md.sha256) - forge.pki.certificateFromPem / certificateToPem - forge.pki.privateKeyFromPem / privateKeyToPem / publicKeyToPem - forge.md.sha256.create() Crypto core is pure Rust on RustCrypto crates already in the lockfile (rsa, x509-cert, der, spki, pkcs1/8, sha2, const-oid, pem, time) - no rcgen, no new heavy deps. Certs are signed sha256WithRSAEncryption; private keys emit PKCS#1 "RSA PRIVATE KEY", public keys SPKI "PUBLIC KEY", matching forge. The cert builder is a real perry JS object so sfw's plain field assignments stay native; methods serialize it via JSON.stringify and sign through the RustCrypto core. DN attribute order is preserved on the parse<->build round-trip so a leaf's issuer DN matches the CA subject DN byte-for-byte (required by `openssl verify`). Wiring: well_known_bindings.toml row (js_node_forge_* / perry_ext_node_forge, date-fns/lru-cache hyphenated convention), NATIVE_MODULES entry, codegen NATIVE_MODULE_TABLE rows + matching API_MANIFEST entries, and the HIR createCertificate->Certificate factory registration. CPU-only: not in binding_needs_shared_tokio. Acceptance: crate unit tests (PEM round-trips, builder JSON parsing, DN round-trip) plus an openssl end-to-end test that builds a CA + leaf and runs `openssl verify -CAfile ca.pem leaf.pem` (=> OK) and `openssl x509 -text` (SANs, EKU serverAuth, sha256WithRSAEncryption all present). * feat(hir): flatten node-forge sub-namespace calls; make PKI binding work end-to-end The perry-ext-node-forge crypto core (added earlier this branch) was inert from compiled TypeScript: forge's API is deeply nested (forge.pki.rsa.generateKeyPair, forge.pki.createCertificate, forge.md.sha256.create), so the call receiver is a Member CHAIN, not a bare native-module Ident — no dispatch arm fired, and an intermediate read like forge.pki hit the unimplemented-API gate (no node-forge symbol 'pki') and deferred a throw. Adds try_node_forge_namespace in perry-hir's call lowering: collapse any member chain rooted at the node-forge default import to its last segment (the method key the codegen NATIVE_MODULE_TABLE rows already use). Runs before the generic namespace / module.Class.staticMethod dispatch, which would otherwise claim the 2-level forge.pki.createCertificate() shape and gate its forge.pki object-read. Also fixes a wrapper fidelity gap: setSubject/setIssuer now store node-forge's { attributes: [...] } DN shape (matching certificateFromPem), so leaf.setIssuer(caCert.subject.attributes) — the sfw idiom — reads the CA subject back and the leaf's issuer DN matches the CA. certificateToPem accepts both the wrapped and bare-array shapes. End-to-end: a CA+leaf generated by the COMPILED binary from real forge.pki.* TypeScript (fixture with no node-forge installed, served by the well-known table) now verifies with the openssl CLI (leaf.pem: OK; issuer=CN=Socket Security CA), correct SANs / keyUsage / extKeyUsage / sha256WithRSAEncryption. Adds an upstream provenance pin (lock-step per #7031) and 3 HIR flattening tests. perry bin 753 pass; node-forge crate 6+1 (incl. openssl e2e); new HIR tests 3/3. (3 pre-existing write-PIC codegen failures on this worktree are unrelated — they fail identically on branches with none of this work.) * docs: key changelog fragment to PR #7033 * fix(node-forge): preserve certificate fidelity and errors --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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
2 participants
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.
Brings the socket-registry fleet's upstream-reference lock-step technique to perry's
well_known_bindings.toml. Every third-party npm package that ships a bundled native wrapper now records the exact upstream release it ports and when the wrapper was last reviewed against it — so a bundled wrapper can't silently drift from the package it claims to reimplement.The pin
The lock-step rule
ported-atmust equalversion. Bumping the pin to a newer upstream without re-reviewing the wrapper against the upstream diff reds the gate — a pin bump can't outrun the review it demands. Enforced in two places: the toml parser inside perry refuses to load a skewed table (so a bad pin can't even ship in the binary), andscripts/binding_pins.mjs --checkgates it in CI (offline, no network).Exemptions (mirrors the fleet: aliases/builtins aren't separately vendored)
node-builtin = true):zlib,events,net,http,https,http2,streams— upstream is Node core, not npm.alias-of):mysql2/promise→mysql2,fetch→node-fetch.@perryts/*,perry/*).A distinct npm package served by a shared wrapper crate is not an alias —
redisand the newiovalkeyboth useperry-ext-ioredisbut are separately published/versioned, so each carries its own pin.scripts/binding_pins.mjsModeled on the fleet's
gitmodules-hash.mts(provisioning) +vendor-actions.mts --check(staleness gate):--set <name> [version]--backfill--check--check --refresh [--soak-days N]--materialize <name>upstream/for port reviewAll 34 third-party bindings are backfilled at their current latest;
--checkis wired intotest.yml.Also:
iovalkeybindingThe Valkey project's fork of ioredis (
valkey-io/iovalkey) — same RESP client API, distinct npm package. Served by the existingperry-ext-ioredissurface, wired throughNATIVE_MODULES, the codegen module-normalization +Redis-constructor lists, and stdlib feature gating. Verified it resolves and compiles from a bareimport Redis from 'iovalkey'with nonode_modules. This PR also annotates everyNATIVE_MODULESentry with a one-line descriptor.Validation
cargo test -p perry --bin perry: 758 passed, 0 failed (5 new: pin parse, lock-step rejection, missing-field rejection, every-entry-pinned, alias-target-exists)node scripts/binding_pins.mjs --check: green (34 pinned, lock-step holds)docs/src/native-libraries/upstream-pins.md+ SUMMARY entryDepends conceptually on #7021 (the skew guard handles unknown feature names when an older checkout is driven by a newer binary), but is independent code.
Summary by CodeRabbit
iovalkeysupport across bindings/dispatch and feature mapping.