crypto: fix property attributes of X509Certificate validFromDate/validToDate

[robobun]

Problem

Reading a property descriptor of X509Certificate.prototype.validFromDate or .validToDate aborts assert-enabled (debug/ASAN) builds. No certificate needed:

Object.getOwnPropertyDescriptor(require("node:crypto").X509Certificate.prototype, "validFromDate");

ASSERTION FAILED: !(attributes & PropertyAttribute::CustomAccessorOrValue)
JavaScriptCore/runtime/PropertyDescriptor.cpp(183) JSC::PropertyDescriptor::setAccessorDescriptor

Anything that walks descriptors hits it too: Object.getOwnPropertyDescriptors(X509Certificate.prototype), util.inspect(proto, { showHidden: true }), test utilities that snapshot descriptors. Release builds are unaffected because get/put check the CustomAccessor bit first.

Cause

The validFromDate and validToDate rows in JSX509CertificatePrototypeTableValues used the attribute mask CustomAccessorOrValue (= CustomAccessor | CustomValue) instead of CustomAccessor. Every neighbouring accessor row (validFrom, validTo, ca, fingerprint, ...) uses CustomAccessor. When getOwnPropertyDescriptor reifies the accessor it strips the CustomAccessor bit, but the stray CustomValue bit survives and trips the assertion in setAccessorDescriptor.

Fix

Use CustomAccessor on both rows, matching the rest of the table.

Verification

• Repro above aborts a debug build on main (exit 134) and is fine after the fix.
• New tests in test/js/node/crypto/x509.test.ts read the descriptors (single and whole-prototype walk) and call the extracted getters on an instance; they abort the debug build before the fix and pass after.
• test/js/node/crypto/x509.test.ts, x509-subclass.test.ts, test-crypto-x509.js, and test-tls-getcertificate-x509.js all pass with the fix.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 13 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: bc4764db-8470-4f3e-8f94-105576b74b1c

📥 Commits

Reviewing files that changed from the base of the PR and between 0ffabf6 and df526e1.

📒 Files selected for processing (2)

• src/jsc/bindings/JSX509CertificatePrototype.cpp
• test/js/node/crypto/x509.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — mechanical two-line attribute fix aligning validFromDate/validToDate with every sibling accessor row.

What was reviewed:

• Confirmed CustomAccessorOrValue is a test mask (= CustomAccessor | CustomValue), not a valid stored attribute; the two changed rows now match the other ~15 CustomAccessor rows in the same table.
• Grepped src/jsc/bindings for other CustomAccessorOrValue uses as a stored attribute — none remain; the two hits in ErrorStackTrace.cpp are correct mask tests.
• New tests exercise both the single-descriptor and whole-prototype-walk paths that previously asserted, plus verify the extracted getters still return correct Date values on an instance.

Extended reasoning...

Overview

Two-line change in src/jsc/bindings/JSX509CertificatePrototype.cpp swapping PropertyAttribute::CustomAccessorOrValue → PropertyAttribute::CustomAccessor on the validFromDate and validToDate HashTableValue rows, plus two regression tests in test/js/node/crypto/x509.test.ts.

CustomAccessorOrValue is defined in JSC as CustomAccessor | CustomValue — a convenience mask for testing whether an attribute is either kind, not a value to set. Setting both bits is contradictory (a slot is either a custom accessor or a custom value). When getOwnPropertyDescriptor reifies the entry, it strips the CustomAccessor bit but the stray CustomValue bit survives into PropertyDescriptor::setAccessorDescriptor, which asserts !(attributes & CustomAccessorOrValue). Release builds don't crash because the get/put fast paths check the CustomAccessor bit first and never reach the assertion.

Security risks

None. This adjusts a static property-attribute flag on two read-only prototype getters. No parsing, no allocation, no user-controlled input paths are touched. The only observable effect is that debug/ASAN builds no longer abort on Object.getOwnPropertyDescriptor(X509Certificate.prototype, 'validFromDate').

Level of scrutiny

Low. This is a copy-paste-class bug where two rows used the wrong enum member while all ~15 sibling rows used the correct one. The fix is purely mechanical — make the outliers match their neighbours. There is no design decision, no new code path, and no release-build behaviour change (the CustomAccessor bit was already set before, so property lookup semantics are identical).

Other factors

• Whole-class check: I grepped src/jsc/bindings for CustomAccessorOrValue — the only remaining uses (ErrorStackTrace.cpp:618,634) are correct bitmask tests (attributes & CustomAccessorOrValue), so no other HashTableValue rows share this bug.
• Tests: The first new test reads descriptors for the two fixed properties and their siblings validFrom/validTo, asserting identical shape — this directly triggers the pre-fix abort and guards against divergence. The full-prototype getOwnPropertyDescriptors walk covers the util.inspect(proto, {showHidden: true}) case from the PR description. The second test verifies the extracted getters still function correctly on an instance (returns a Date equal to direct property access), guarding against accidentally breaking the accessor while fixing the attribute.
• PR verification claims are consistent with the change: the repro aborts on main (exit 134) and passes after; adjacent x509 test files still pass.

[robobun]

CI status for build 89621: the only hard failure is test/js/node/worker_threads/worker-transfer-terminate-stress.test.ts (SIGABRT on the Debian 13 x64-asan lane). That failure is present on main as well and is unrelated to this change; it has been reported for separate triage. Every other failing file in the build is a known-flaky test that passed on retry or passed when run alone.

The new x509 descriptor tests passed on all lanes, including x64-asan, which is the lane where the original assertion fired. The diff is ready for review.