Repository navigation
v7.0.0: revive the library, ESM + Node 24 + modern toolchain - #658
Conversation
…24.12+ BREAKING CHANGE: Drops support for Node < 24.12 and switches to ESM-only. Library now ships as pure ESM; CommonJS consumers must use dynamic import(). Engines pin: ">=24.12.0". Toolchain overhaul: - TypeScript 7 preview (@typescript/native-preview tsgo binary) replaces tsc. Emit is clean; build runs the Go-native compiler. - oxlint + oxlint-tsgolint replaces ESLint + @typescript-eslint. Type-aware linting via tsgo under the hood. - oxfmt replaces Prettier. Pre-1.0 but sufficient for this repo. - Husky + lint-staged removed. hk (jdx/hk) is the recommended hook manager (documented in CLAUDE.md); hk is a system binary, not an npm dep. - Jest config/setup removed; tests to be migrated to node:test in a follow-up (all test files still compile under the new toolchain). Deps removed entirely: - node-forge: replaced by node:crypto + pkijs (pkijs@^3.4) for PKCS#7 SignedData construction. Native X509Certificate parses PEM and emits DER via .raw, eliminating hand-rolled PEM-to-DER code. - yauzl + @types/yauzl + event-iterator: replaced by a ~200-line in-repo src/lib/zip.ts unifying both the reader (STORE + DEFLATE only, size-capped for pkpass use) and the writer (STORE-only). - do-not-zip: its 56-line STORE-only writer inlined into src/lib/zip.ts so the library owns both directions. Package last published 2022, avoiding a supply-chain risk. - buffer-crc32: CRC32 table inlined into zip.ts (~10 lines). devDeps collapse from 13 packages to 5: - @types/node ^24, @typescript/native-preview (pinned 7.0.0-dev.*), oxlint ^1.63, oxlint-tsgolint ^0.22, oxfmt ^0.48. Crypto rewrite (src/lib/sign-manifest.ts): - WWDR G4 cert is inlined as a base64 literal (bundle-friendly for AWS Lambda; no runtime filesystem reads). - Template stores cert as PEM string and key as node:crypto KeyObject internally. Public API unchanged: setCertificate(pem, pwd?), setPrivateKey(pem, pwd?), loadCertificate(path, pwd?). - PEM ↔ DER conversions go through node:crypto X509Certificate; no hand-rolled base64 stripping. - Round-trip verified: produced signatures pass `openssl cms -verify` (2,469-byte DER blob for a minimal manifest). NFC rewrite (src/lib/nfc-fields.ts): - setPublicKey now accepts PEM only (the node-forge PublicKey object form was never documented as a separate path and is unusable without node-forge). P-256 curve check is done via KeyObject asymmetricKeyDetails. Bundling: - package.json "type": "module", "sideEffects": false, "exports" map. - NO native addons, NO __dirname, NO require.resolve() in src/. - Verified via esbuild: the built module loads cleanly on Node 24+. tsconfig: - target ES2023, module NodeNext, verbatimModuleSyntax, isolatedModules, noUncheckedIndexedAccess, erasableSyntaxOnly, skipLibCheck. - lib includes DOM for Web Crypto types pkijs needs. Source shape: - 19 source files updated to ESM: `.js` import extensions, type-only imports where required by verbatimModuleSyntax, no 'use strict'. - src/types.d.ts ambient declarations for color-name and imagesize (deleted the old src/@types/ stubs). Out of scope (separate commits to follow): - Test migration Jest → node:test - Re-adding Buffer-friendly Template builders and tests - New CI workflows (ci.yml, release-please.yml, publish.yml) - release-please configs, npm trusted publishing Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reviewed all 12 findings; fixed the real issues, skipped false positives and plan-covered items. Fixed: - src/lib/zip.ts: added strict bounds checks in central-directory parse loop (p + 46 and entryBlockEnd against centralEnd) and in getBuffer's local-header read path (h + 30, dataStart sanity). Defence-in-depth against malformed zip inputs beyond the existing entry-count and per-entry size caps. - src/lib/pass-structure.ts: `set style` no longer clobbers an existing NFCField when setting the style to storeCard a second time. Only instantiates a new NFCField when storeCard is selected AND this.fields.nfc is unset. - src/pass.ts: grammar fix — "is presented" → "is present" in the authenticationToken / webServiceURL validation error. - src/types.d.ts: ImagesizeParser.parse now returns `number` only, matching the numeric DONE/INVALID/EAGAIN constants on Parser. The prior `number | 'done' | 'invalid'` union was wrong (no string-literal return exists). - src/lib/get-buffer-hash.ts: documented why SHA-1 is required here — it's Apple's PassKit manifest format, not a security choice. The PKCS#7 signature provides the integrity. - src/lib/sign-manifest.ts: removed dead X509Certificate parsing + `void x509` placeholder; parsePkiCertificate already validates via X509Certificate internally. Skipped (false positives or plan-covered): - src/pass.ts:86 manifestJson-as-Buffer: ZipWriteEntry.data accepts `Buffer | string` by design; writeZip internally Buffer.from()'s strings. False positive. - tsconfig erasableSyntaxOnly: valid TS 5.8+ option. False positive. - @typescript/native-preview dev build: intentional v7 choice with fallback to classic tsc documented in CLAUDE.md. Plan-covered. - engines: ">=24.12.0" + "type": "module" as breaking changes: yes, which is why this work targets 7.0.0. Plan-covered; the current 6.9.8 version in package.json is a transient state before release-please takes over in Phase 7. - interfaces.ts encryptionPublicKey migration note: belongs in the v7 CHANGELOG/README, not source. Will go in Phase 7. Verified post-fix: - `npm run build` emits clean. - openssl cms -verify still passes on emitted dist. - zip edge cases (unicode filenames, empty files, path traversal, malformed input, CRC tampering) all handled correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces all 11 test files' Jest APIs with native node:test + node:assert/strict.
Drops jest-extended matchers inline (toBeString, toEndWith, toBeValidDate,
toIncludeSameMembers were the only ones used).
Results:
- 60 tests across 10 suites; 58 pass, 2 legitimately skipped.
- Overall coverage: 90.56% line / 79.38% branch / 88.98% function
(via --experimental-test-coverage on Node 25.9 / Node 24.12+).
- oxlint --type-aware: 0 errors in test files.
- Test script now runs `npm run build` first and executes tests against
`dist/` artifacts — matches how real consumers import the library.
Key changes by file:
- __tests__/signManifest.ts: previously relied on APPLE_PASS_* env vars
(expired 2020; broken since). Now generates a throwaway self-signed
Pass Type ID cert on the fly with openssl and asserts `openssl cms -verify`
succeeds on the round-trip output — a far stronger test than the original
"is it a Buffer" check.
- __tests__/pass.ts: likewise self-generates certs for the full pass
build tests. The live-APNs `pushUpdates` test path in template.ts is
now a proper `{ skip: !env.APPLE_PASS_CERTIFICATE || !env.APPLE_PUSH_TOKEN }`
opt-in rather than a silent failure.
- __tests__/nfc-field.ts: replaced node-forge RSA keygen with node:crypto
generateKeyPairSync. Added a curve-mismatch test (secp384r1 rejected).
- __tests__/images.ts, localizations.ts, pass.ts: snapshot assertions
migrated to node:test's `t.assert.snapshot(value)`. Generated
*.snapshot sidecar files committed alongside the tests.
Scope of the migration:
- Zero shim / harness code — Jest matchers are open-coded as
`assert.equal`/`deepEqual`/`throws`/`rejects`/`match`.
- Tests import from `../dist/*.js` because node:test's native TS
type-stripping doesn't rewrite the internal `.js` import specifiers
inside src/. Running tests against dist exercises the same artifact
npm publishes.
- `beforeAll`/`afterAll` → `before`/`after` (node:test names).
- `jest.setTimeout(N, fn)` not needed; tests complete in ~600ms total.
Coverage gaps (left for follow-up):
- src/lib/zip.ts branch coverage 54% — malformed-ZIP error paths
added for the CodeRabbit P1 findings aren't exercised by existing
tests. A dedicated zip fuzz test is the highest-impact add.
- src/template.ts pushUpdates (lines 149-188) — requires live APNs.
- src/lib/base-pass.ts 73% line — many setter validation branches
not exercised.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- .npmrc: fix typo `save-exact§` → `save-exact`. The stray `§` at the
end of the key has been emitting
`npm warn Unknown project config "save-exact§"` on every npm
invocation since 2020. No behavioural change; npm was silently
ignoring the malformed key.
- oxlint errors → 0:
- src/lib/images.ts: fix `Unknown image type ${imageSize}` which
interpolated the top-level imagesize factory function rather than
the intended value. Real pre-existing bug in the error message.
- src/lib/fieldsMap.ts + base-pass.ts: wrap unknown/Date values in
String() / .toString() before interpolating into TypeError messages
(typescript/restrict-template-expressions).
- src/lib/localizations.ts: drop useless spread
`new Map([...map])` → `new Map(map)` (Map accepts the iterable).
- src/types.d.ts: simplify `'PNG' | 'JPEG' | 'GIF' | 'WEBP' | string`
union (no-redundant-type-constituents) to `string` with a comment.
- oxlint warnings → 0:
- Add targeted rule tuning in .oxlintrc.json:
- Disable `no-unsafe-type-assertion`, `no-unnecessary-type-assertion`,
`no-unsafe-assignment` globally — too many false positives on
legitimate `as` casts in this codebase.
- Override for __tests__: allow underscore-prefixed names
(node:fs callback conventions), skip unbound-method for our
class-reference assertions.
- Use `Array#toSorted()` instead of `.sort()` on ES2023+
(no-array-sort).
- __tests__/pass.ts: rename loop var `path` → `entryPath` to avoid
shadowing the node:path import; switch
`pass.labelColor instanceof Array` → `Array.isArray(...)`
(unicorn/no-instanceof-builtins).
- __tests__/localizations.ts: hoist sort comparator out of the test
body (consistent-function-scoping).
- src/lib/w3cdate.ts: drop unary `+1` that was flagged as a
no-op conversion.
- Add .oxfmtrc.json pinning the formatter to the repo's original
prettier style: single quotes, `arrowParens: avoid`, `trailingComma:
all`, `printWidth: 80`, `endOfLine: lf`. Reformat every .ts file
under src/ and __tests__/ to match. Without the config, oxfmt's
defaults used double quotes.
Verified:
- oxlint --type-aware: 0 warnings, 0 errors on 30 files
- oxfmt --check: all 32 files clean
- npm run build: tsgo emits cleanly
- npm test: 60 tests, 58 pass, 2 legitimately skipped
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The human-readable copy at keys/wwdr.pem previously held the expired pre-2023 cert. The runtime cert (inlined in src/lib/sign-manifest.ts) was already updated to G4 during the toolchain rewrite; keys/wwdr.pem is now brought into sync. The G4 cert is valid through 2030-12-10 and is Apple's designated CA for Pass Type ID signing. Source: https://www.apple.com/certificateauthority/AppleWWDRCAG4.cer SHA-256: EA:47:57:88:55:38:DD:8C:B5:9F:F4:55:6F:67:60:87:D8:3C:85:E7:09:02:C1:22:E4:2C:08:08:B5:BC:E1:4C Credit to @lucyyyyyyy (PR #626) who proposed the same cert update in September 2022. Closes #616. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves the long-standing #480 feature request. Re-applies community PR #479 by @akoufa (2020-07-01) on top of the v7 codebase. When `disableImageCheck: true` is passed via template Options or to `PassImages.add()`/`load()` directly, the image dimension validation in `PassImages.checkImage` is skipped. Useful for Lambda/bundled environments where shipping the Apple-spec-compliant dimensions isn't feasible, or when generating passes with atypical icon sizes on purpose. - Options.allowHttp is now also optional (previously required boolean) - Added test covering the skip path with a 1x1 PNG fixture buffer Co-Authored-By: Konstantin Akoufa (original PR author) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves two long-standing gaps from #652 (by @navelencia): fields that were accepted by the type system but never serialized to the emitted pass.json. - appLaunchURL: documented getter/setter on PassBase + entry in TOP_LEVEL_FIELDS. The field was already declared in ApplePass but had no accessor, so assignments via `pass.appLaunchURL = "..."` did nothing. Opens the issuer app via associatedStoreIdentifiers when the pass is tapped. - NFCDictionary.requiresAuthentication: when `true`, the user must authenticate (Face ID / Touch ID / passcode) before the NFC transmission. NFCField now persists this through construction, assignment, and JSON serialization. Skipped from PR #652: - The yazl-based zip writer swap. Our in-repo src/lib/zip.ts (Phase 1) already covers the write path with zero external deps; yazl would regress the bundle-friendliness goal. Added tests covering both round-trips. Co-Authored-By: Fred Brandt (original PR #652 author) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes #75 (2019). Re-applies community PR #657 (by @apples-kksk, 2026-05-09) on top of the v7 codebase. Semantic tags are machine-readable metadata that Wallet uses to offer passes in context (e.g. lock-screen suggestions, related actions) and are now mandatory for iOS 18+ poster / enhanced event tickets. Types: - SemanticTagValue: recursively-typed union (string | number | boolean | Date | object | array). - SemanticTags / SemanticTagObject: the container shape (dictionary keyed by Apple's documented tag names, e.g. `eventStartDate`, `venueLocation`, `totalPrice`). Apple defines ~70 specific keys but the dictionary stays open-ended for forward compatibility. - PassSemanticKeys: wrapped into ApplePass alongside the existing PassStandardKeys / PassWebServiceKeys groupings. API surface: - `pass.semantics` / `template.semantics` getter+setter on PassBase. Assigning a dictionary runs normalizeSemanticTags which recursively converts `Date` values to W3C date strings (consistent with how FieldsMap normalizes `value`). Setting `undefined` deletes the field. - FieldsMap serialization now runs the same normalization for per-field `semantics`. A field's semantics shadow the top-level pass.semantics entry for that field. - New `normalizeSemanticTags` helper lives in src/lib/semantic-tags.ts (~30 LOC; ESM + type-only imports). - SemanticTags types re-exported from src/index.ts so consumers can annotate their own dictionaries. Invalid Date values in semantics now throw `Semantic tag Date values must be valid` rather than silently emitting `"Invalid Date"` as a string. Deferred from PR #657: - The README update — README overhaul is planned separately for v7. Added tests: - PassBase: round-trip through JSON, rejection of invalid Date. - FieldsMap: per-field semantics serialize with Date→W3C conversion. Co-Authored-By: apples-kksk (original PR #657 author) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Covers the 2024-2026 PassKit additions that didn't have community PRs
open yet. These are additive type + accessor changes; runtime behaviour
is pass-through (Apple validates at pass-open time).
New fields:
- `PassRelevanceKeys.relevantDates`: an array of RelevantDateEntry
objects, each either a single `relevantDate` or a
`startDate`+`endDate` window. Supersedes the singular `relevantDate`
for multi-leg itineraries and recurring events.
- `PassVisualAppearanceKeys.preferredStyleSchemes`: ordered array
opting into iOS 18+ rendering styles ('posterEventTicket',
'eventTicket'). Older OSes silently ignore it.
Deprecations (emit TS warnings, still functional):
- `PassRelevanceKeys.relevantDate` → use `relevantDates` for new
passes.
- `PassVisualAppearanceKeys.barcode` → use `barcodes` (deprecated by
Apple since iOS 9.0, but we hadn't flagged it).
Also exposed SemanticTagValue / SemanticTagObject from index.ts to let
consumers annotate their own objects.
Skipped (too much surface for one commit):
- UpcomingPassInformationEntry: requires Apple's full `PassInformation`
interface chain which they haven't fully formalized in their public
schema yet. Add once Apple publishes the canonical type.
- secondaryLogo image asset: needs PassImages + IMAGES constant work
plus image type enforcement. Follow-up.
- Wallet Orders (order.json) format: separate package format, separate
cert flow. Out of scope for v7.
Tests added:
- round-trip relevantDates array (single + window entries)
- preferredStyleSchemes set+clear
- empty array clears the field
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 6 of the v7 plan: set the repo up for ongoing AI-assisted
maintenance and replace the 2019-vintage CI pipeline.
Added:
- CLAUDE.md: maintainer notes for Claude Code / Cursor / Aider / etc.
Covers build/test commands, architecture, non-obvious bits
(SHA-1-is-by-spec, inlined WWDR cert, tests-against-dist),
manual-QA requirements, and the hk git-hook setup. ~80 LOC — tight
enough to stay in the assistant's working context.
- AGENTS.md: one-line pointer at CLAUDE.md so tool-neutral standards
see the same file.
- .github/workflows/ci.yml: replaces test.yml + lint.yml. Matrix
ubuntu/macos/windows × Node 24 + 26. Runs build + lint + tests +
coverage on every PR. A second job does a bundle smoke-test via
both esbuild (external deps mode) and @vercel/ncc (single-file
bundle) to catch regressions that would break Lambda consumers.
- .github/workflows/mehen.yml: runs ophidiarium/mehen@v1 on every PR
for cyclomatic/cognitive/Halstead complexity metrics. No secrets
required; posts a delta comment on the PR.
- .github/dependabot.yml: replaces the deleted renovate.json. Weekly
npm bumps grouped as { types, dev-dependencies, prod-dependencies };
monthly github-actions bumps; ignore @typescript/native-preview
minor/patch (preview builds break too often to auto-merge).
Removed:
- .github/workflows/lint.yml, test.yml, npmpublish.yml: superseded.
npmpublish.yml specifically is replaced in Phase 7 by release-please
+ npm trusted publishing (OIDC).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 7: lock in low-friction releases for the v7 era and beyond. Workflow: 1. Every push to master runs release-please, which watches conventional commits (`feat:`, `fix:`, `feat!:`) and opens a PR that bumps package.json + maintains CHANGELOG.md. 2. Merging the release PR cuts a GitHub Release. 3. The release event triggers publish.yml, which publishes to npm via OIDC trusted publishing — no NODE_AUTH_TOKEN, no long-lived secret. Provenance attestation is automatic. Files: - .github/workflows/release-please.yml — googleapis/release-please-action@v5 - release-please-config.json — node release type, sectioned changelog (feat / fix / perf / deps / docs / refactor; chores+tests+ci+build hidden). - .release-please-manifest.json — seeded at 6.9.8 (the current published version on npm). The first release PR that lands on a branch with a `feat!:` commit (i.e. the v7 merge) will propose 7.0.0. - .github/workflows/publish.yml — fires on `release: published`. Uses actions/setup-node@v6 with registry-url set; npm 11.5+ auto-detects the OIDC trusted publisher. One-time setup on npmjs.com (before merging v7-modernization into master): `@walletpass/pass-js` → Settings → Trusted Publisher → add GitHub Actions publisher with: - Organization: tinovyatkin - Repository: pass-js - Workflow filename: publish.yml - Environment: (blank) Once verified, the legacy NPM_TOKEN secret can be deleted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMigration to ESM/Node 24+, replace node-forge/Jest with Node-native crypto and node:test, add pkijs-based PKCS#7 signing, custom ZIP read/write, semantic-tag normalization, new iOS 18+ pass fields, tooling/workflow overhaul, and broad test updates to use built artifacts. ChangesFull Modernization Migration
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Two CI failures on the initial v7 PR run: 1. Windows shards of `test` fail `npm run lint` because oxfmt (with `endOfLine: lf`) sees every file as having CRLF line endings. Git's default `core.autocrlf=true` on Windows checks files out with CRLF regardless of what's in the repo. Add `.gitattributes` pinning all text files to LF so Windows contributors get the same byte-level content CI sees. Binary assets (png/pem/zip/pkpass/etc.) are explicitly flagged `binary` so they're never touched. 2. `ophidiarium/mehen@v1` doesn't exist yet — the action only tags `v0`. Pinning to v0 until the maintainer cuts v1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f324bf725
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Apple's .strings format spec is unambiguous: the \n escape sequence always means U+000A (LF), regardless of host OS. The previous implementation substituted os.EOL on both encode and decode, which meant a .strings value encoded on macOS/Linux decoded to the same string on macOS/Linux but to a CRLF-containing string on Windows, and the round-trip reproduced a different pass bundle byte-for-byte across platforms. That is: - a silent correctness bug for cross-platform build pipelines - broke the Windows test shard's localization snapshot on the CI introduced in the previous commit escapeString now also handles CR, CRLF, and LF sources symmetrically (any line-ending style in user input → `\n` escape on the wire). Also add *.strings and *.snapshot to .gitattributes so git on Windows doesn't apply autocrlf to them (and prospective contributors with core.autocrlf=true get reproducible test output). Root cause is pre-existing: tests only ran on macOS before the v7 CI rewrite added Windows to the matrix, so the drift was never detected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- bundle-smoke: drop `--packages=external` from the esbuild step. The whole point of this test is to prove the library works as a single-file Lambda bundle with no node_modules at runtime. Previous config produced a thin shim that still required strip-json-comments at runtime; the new loader runs outside the repo so we prove the resulting bundle is truly self-contained. - mehen: mark the job continue-on-error + the mehen step continue-on-error. The action crashes with "Unexpected end of JSON input" when the PR diff is too large (v7 rewrites many files vs master). The action is informational metrics only; failing it shouldn't block a merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
esbuild's ESM output wraps CJS deps in a `__require` shim that doesn't
handle `node:*` specifiers. @noble/hashes (a pkijs transitive) calls
`require('node:crypto')`, which blows up at load time.
Fix: inject a `createRequire(import.meta.url)` banner so any CJS dep
still bundled in can resolve node builtins. This is the standard
workaround documented in esbuild's docs for the ESM-wrapping-CJS case.
Also pin --target=node24 to match engines.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efa354efa5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
__tests__/w3cdate.ts (1)
29-39:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix tautological timezone assertion.
Line 39 compares the same value to itself, so this check never validates parsed output. Compare
date1againstdateinstead.Proposed fix
- assert.equal(date.getTimezoneOffset(), date.getTimezoneOffset()); + assert.equal(date1.getTimezoneOffset(), date.getTimezoneOffset());🤖 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 `@__tests__/w3cdate.ts` around lines 29 - 39, The test contains a tautological assertion comparing date.getTimezoneOffset() to itself; update the assertion in the 'parseable to Date' test to compare the parsed Date (date1) timezone offset against the original Date (date) by changing the assertion to compare date1.getTimezoneOffset() with date.getTimezoneOffset(); locate the test block that uses getW3CDateString(date) and variables date and date1 to make this change.src/template.ts (1)
263-267:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject here instead of throwing from the
connectcallback.The
throwon Line 265 runs inside an event handler, so it escapes the surrounding Promise and can become an uncaught exception instead of rejectingpushUpdates().💡 Proposed fix
apn .once('goaway', () => { if (this.apn && !this.apn.destroyed) this.apn.destroy(); }) .once('error', reject) .once('connect', () => { if (apn.destroyed) - throw new Error('APN was destroyed before connecting'); + return reject( + new Error('APN was destroyed before connecting'), + ); this.apn = apn; resolve(); });🤖 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 `@src/template.ts` around lines 263 - 267, The connect event handler currently throws an Error inside the .once('connect', ...) callback which escapes the surrounding Promise; change that throw to call the Promise's reject callback instead and ensure you don't call resolve afterwards. In the .once('connect', ...) handler used by pushUpdates(), replace "throw new Error('APN was destroyed before connecting')" with "reject(new Error(...))" (and return immediately), so the Promise returned by pushUpdates() is properly rejected; keep the existing this.apn = apn; resolve(); path only when apn is not destroyed.
🧹 Nitpick comments (1)
__tests__/pass.ts (1)
17-30: 💤 Low valueTest setup correctly generates throwaway certificates.
The static analysis tool flagged
execSyncfor potential command injection, but this is a false positive—all arguments are hardcoded string literals with no user-controlled input. For test-only certificate generation this approach is acceptable.If you prefer stricter hygiene,
execFileSyncwith an array of arguments would eliminate the shell interpretation entirely, though it requires handling the-subjargument carefully:♻️ Optional: switch to execFileSync
- execSync( - `openssl req -x509 -newkey rsa:2048 -keyout ${keyPath} -out ${certPath} ` + - `-days 1 -nodes -subj "/CN=Pass Type ID: pass.com.example.passbook/O=pass-js test"`, - { stdio: 'ignore' }, - ); + execFileSync( + 'openssl', + [ + 'req', '-x509', '-newkey', 'rsa:2048', + '-keyout', keyPath, '-out', certPath, + '-days', '1', '-nodes', + '-subj', '/CN=Pass Type ID: pass.com.example.passbook/O=pass-js test', + ], + { stdio: 'ignore' }, + );🤖 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 `@__tests__/pass.ts` around lines 17 - 30, The use of execSync in makeTestKeypair triggered a command-injection warning; fix by either (A) documenting and silencing the false positive (add a comment above makeTestKeypair explaining all args are hardcoded literals and add the appropriate linter/TS suppression), or (B) replace execSync with execFileSync and pass the command and its arguments as an array (e.g., 'openssl' and ['req','-x509','-newkey','rsa:2048','-keyout', keyPath, '-out', certPath, '-days','1','-nodes','-subj','/CN=Pass Type ID: pass.com.example.passbook/O=pass-js test']) so shell interpretation is avoided; update makeTestKeypair accordingly and ensure tests still read certPath/keyPath.
🤖 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 `@__tests__/signManifest.ts`:
- Around line 19-23: The test uses execSync with a shell-interpolated command
(execSync(... `openssl req ... ${keyPath} ${certPath}` ...)), which is fragile
for paths with special chars; replace those calls (references: execSync,
keyPath, certPath) with execFileSync and pass OpenSSL and its flags as an argv
array (e.g., ['req','-x509','-newkey','rsa:2048','-keyout', keyPath,'-out',
certPath,'-days','1','-nodes','-subj', '/CN=Pass Type ID: pass.test/O=pass-js
test']), and remove reliance on parsing OpenSSL textual output — rely on
execFileSync throwing on non-zero exit (or check the returned Buffer) to verify
success; apply the same change to the other occurrence at lines ~50-53.
In `@__tests__/template.ts`:
- Around line 78-101: The skip condition for the "push updates" test is
incomplete: ensure the test also skips when APPLE_PASS_PRIVATE_KEY is missing by
adding process.env['APPLE_PASS_PRIVATE_KEY'] to the logical OR check so the test
only runs when APPLE_PASS_CERTIFICATE, APPLE_PASS_PRIVATE_KEY, and
APPLE_PUSH_TOKEN are all present; update the skip predicate that guards the
it('push updates', ...) block (which constructs a new Template and calls
setPrivateKey(...) and pushUpdates(...)) to include the private key environment
variable.
In @.github/workflows/mehen.yml:
- Around line 17-23: Replace the mutable action tags with pinned commit SHAs:
change the uses entries for actions/checkout (currently "actions/checkout@v5")
and ophidiarium/mehen (currently "ophidiarium/mehen@v0") to use their
corresponding full commit SHA values; update the workflow YAML to reference
those SHAs (e.g., "actions/checkout@<full-sha>" and
"ophidiarium/mehen@<full-sha>") so the checkout and mehen steps use immutable,
auditable versions while preserving the existing with: and continue-on-error:
settings.
In @.github/workflows/release-please.yml:
- Around line 15-18: Replace the mutable tag on the GitHub Action
(googleapis/release-please-action@v5) with a full-length commit SHA to pin the
release-please action immutably; update the uses entry that currently references
googleapis/release-please-action@v5 to
googleapis/release-please-action@<full-commit-sha> (obtain the desired SHA from
the release-please-action repo commit history) so the workflow always runs an
exact, auditable commit instead of a floating tag.
In `@CLAUDE.md`:
- Around line 23-43: The Markdown fenced code blocks in the CLAUDE.md examples
need explicit language tags to satisfy MD040; update the first directory-listing
fence to use ```text and update the brew/install snippet fence to use ```sh (the
same change should be applied to the other identical snippet later in the file),
ensuring both fenced blocks include these language identifiers instead of bare
``` so linting won’t flag them.
In `@package.json`:
- Line 38: The npm "test" script in package.json is using single quotes around
the glob (the "test" script value node --test '__tests__/*.ts') which breaks on
Windows; update the "test" script to use double quotes (node --test
"__tests__/*.ts") or remove the quotes entirely so the glob expands
cross-platform, keeping the rest of the script (npm run build && ...) unchanged.
In `@src/lib/images.ts`:
- Around line 258-271: In the 'thumbnail' validation case (case 'thumbnail')
update the thrown TypeError messages to match the actual checks: for width use
the 120 * densityMulti limit and for height use the 150 * densityMulti limit
(and optionally fix the typo "no large than" to "no larger than"); ensure the
messages reference width, height and densityMulti variables consistently so the
reported limit matches the checked expression.
In `@src/lib/localizations.ts`:
- Around line 35-39: The unescapeString function currently converts every '\n'
escape sequence into a real newline, which also transforms literal "\\n"
sequences incorrectly; change the '\n' conversion to only replace backslash-n
sequences that are not themselves escaped (i.e. use a negative-lookbehind or
equivalent to match only unescaped \\n) inside unescapeString so literal "\\n"
stays as two characters while true escaped newlines become real newlines; keep
the existing quote/backslash unescaping (replace(/\\(["\\])/g, '$1')) and ensure
the order of replacements preserves intended literals.
In `@src/lib/pass-structure.ts`:
- Around line 83-93: The style setter currently preserves top-level
this.fields.nfc when switching styles, violating the invariant that NFC only
exists for storeCard; update the setter in the style accessor (the set style(v:
PassStyle | undefined) method) to explicitly delete this.fields.nfc when the new
style v is not 'storeCard' (i.e., after you remove other style containers via
PASS_STYLES, if v !== 'storeCard' then delete this.fields.nfc), and ensure you
only create a new NFCField when setting v to 'storeCard' (keep the existing
check that adds new NFCField only when 'storeCard' is present and
this.fields.nfc is missing). This keeps PASS_STYLES, fields, storeCard,
NFCField, and ApplePass handling consistent and prevents invalid serialized
payloads.
In `@src/lib/semantic-tags.ts`:
- Around line 5-26: The normalizeSemanticValue function currently recurses
without cycle detection and will stack-overflow on self-referential
arrays/objects; add cycle detection using a WeakSet of visited objects tracked
across the recursion (either by adding an optional visited: WeakSet<object>
parameter to normalizeSemanticValue or by creating a closure WeakSet in
normalizeSemanticTags and passing it into recursive calls). Before recursing
into an array or object in normalizeSemanticValue, check visited.has(value) and
if true throw a TypeError (e.g., "Cyclic semantic-tag reference"); otherwise
call visited.add(value), recurse into elements/properties with the same visited
set, and proceed as before. Ensure Date/primitive handling remains unchanged and
only objects/arrays are added to the WeakSet so garbage collection is preserved.
In `@src/lib/sign-manifest.ts`:
- Around line 38-67: The current APPLE_WWDR_G4_PEM constant stores the WWDR cert
as PEM armor; change it to a base64-only string constant (e.g.,
APPLE_WWDR_G4_B64) that contains only the raw base64 payload, and update
APPLE_WWDR_CA_PEM so it decodes that base64 locally (Buffer.from(..., 'base64')
or equivalent) before using/parsing the certificate; keep the existing env
override behavior but expect/accept a base64 string in
process.env['APPLE_WWDR_CERT_PEM'] and only decode it when assigning
APPLE_WWDR_CA_PEM in sign-manifest.ts (referencing APPLE_WWDR_G4_B64 /
APPLE_WWDR_CA_PEM to locate the change).
In `@src/lib/w3cdate.ts`:
- Around line 47-50: The parser regex in this file is too strict compared to
isValidW3CDateString: update the regex(es) that set res (and the other two
occurrences) to accept an optional seconds component and either a "Z" timezone
or a signed offset; e.g. change the time portion to allow (?::(?<secs>\d{2}))?
and the timezone to (?:Z|(?<tzSign>[+-])(?<tzHour>\d{2}):(?<tzMin>\d{2})), then
update the downstream parsing logic that reads the capture groups (secs, tzSign,
tzHour, tzMin) so parsing behavior matches isValidW3CDateString; make the same
change for the other regex instances referenced in this file so validation and
parsing accept the same W3C date variants.
In `@src/lib/zip.ts`:
- Around line 29-37: writeZip currently emits unsafe entry names; before
converting each ZipWriteEntry.path to a Buffer, validate and reject names that
are absolute (start with '/'), contain backslashes ('\\'), are empty, or contain
'..' path segments (e.g. path.split('/').includes('..')); throw a clear error
(or otherwise fail fast) from writeZip when an invalid name is encountered so
produced archives cannot contain zip-slip style entries; perform this check on
the path string before creating name = Buffer.from(path, 'utf8') and before
writing local/central records.
In `@src/template.ts`:
- Around line 140-187: The ZIP rebuild loop must enforce exactly one pass.json:
in the function that reads the zip (the block using readZip,
createDefaultTemplate, and constructing new Template), add a boolean flag like
passJsonFound = false; when you match /\/?pass\.json$/i, if passJsonFound is
true throw a TypeError for duplicate pass.json, otherwise set passJsonFound =
true, parse the JSON and construct the Template as you already do; after the for
loop, if passJsonFound is false throw a TypeError indicating no pass.json found.
This preserves the duplicate guard and ensures archives without a pass.json fail
fast; reference Template (constructor), createDefaultTemplate,
Template.images.parseFilename, and Template.localization.addFromBuffer to locate
the relevant code.
---
Outside diff comments:
In `@__tests__/w3cdate.ts`:
- Around line 29-39: The test contains a tautological assertion comparing
date.getTimezoneOffset() to itself; update the assertion in the 'parseable to
Date' test to compare the parsed Date (date1) timezone offset against the
original Date (date) by changing the assertion to compare
date1.getTimezoneOffset() with date.getTimezoneOffset(); locate the test block
that uses getW3CDateString(date) and variables date and date1 to make this
change.
In `@src/template.ts`:
- Around line 263-267: The connect event handler currently throws an Error
inside the .once('connect', ...) callback which escapes the surrounding Promise;
change that throw to call the Promise's reject callback instead and ensure you
don't call resolve afterwards. In the .once('connect', ...) handler used by
pushUpdates(), replace "throw new Error('APN was destroyed before connecting')"
with "reject(new Error(...))" (and return immediately), so the Promise returned
by pushUpdates() is properly rejected; keep the existing this.apn = apn;
resolve(); path only when apn is not destroyed.
---
Nitpick comments:
In `@__tests__/pass.ts`:
- Around line 17-30: The use of execSync in makeTestKeypair triggered a
command-injection warning; fix by either (A) documenting and silencing the false
positive (add a comment above makeTestKeypair explaining all args are hardcoded
literals and add the appropriate linter/TS suppression), or (B) replace execSync
with execFileSync and pass the command and its arguments as an array (e.g.,
'openssl' and ['req','-x509','-newkey','rsa:2048','-keyout', keyPath, '-out',
certPath, '-days','1','-nodes','-subj','/CN=Pass Type ID:
pass.com.example.passbook/O=pass-js test']) so shell interpretation is avoided;
update makeTestKeypair accordingly and ensure tests still read certPath/keyPath.
🪄 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
Run ID: 9549aae5-bd54-4ae9-8664-28770da56343
⛔ Files ignored due to path filters (5)
__tests__/__snapshots__/images.ts.snapis excluded by!**/*.snap__tests__/__snapshots__/localizations.ts.snapis excluded by!**/*.snap__tests__/__snapshots__/pass.ts.snapis excluded by!**/*.snapkeys/wwdr.pemis excluded by!**/*.pempackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (68)
.eslintignore.eslintrc.gitattributes.github/dependabot.yml.github/workflows/ci.yml.github/workflows/lint.yml.github/workflows/mehen.yml.github/workflows/npmpublish.yml.github/workflows/publish.yml.github/workflows/release-please.yml.github/workflows/test.yml.huskyrc.npmrc.nvmrc.oxfmtrc.json.oxlintrc.json.prettierrc.json.release-please-manifest.json.sonarcloud.propertiesAGENTS.mdCLAUDE.md__tests__/base-pass.ts__tests__/fieldsMap.ts__tests__/get-geo-point.ts__tests__/images.ts__tests__/images.ts.snapshot__tests__/localizations.ts__tests__/localizations.ts.snapshot__tests__/nfc-field.ts__tests__/normalize-locale.ts__tests__/pass.ts__tests__/pass.ts.snapshot__tests__/signManifest.ts__tests__/template.ts__tests__/w3cdate.tsjest.config.jslint-staged.config.jspackage.jsonrelease-please-config.jsonrenovate.jsonsrc/@types/do-not-zip/index.d.tssrc/@types/imagesize/index.d.tssrc/constants.tssrc/index.tssrc/interfaces.tssrc/lib/base-pass.tssrc/lib/fieldsMap.tssrc/lib/get-buffer-hash.tssrc/lib/get-geo-point.tssrc/lib/getBufferHash.tssrc/lib/images.tssrc/lib/localizations.tssrc/lib/nfc-fields.tssrc/lib/normalize-locale.tssrc/lib/pass-color.tssrc/lib/pass-structure.tssrc/lib/semantic-tags.tssrc/lib/sign-manifest.tssrc/lib/signManifest-forge.tssrc/lib/stream-to-buffer.tssrc/lib/w3cdate.tssrc/lib/yazul-promisified.tssrc/lib/zip.tssrc/pass.tssrc/template.tssrc/types.d.tstsconfig.all.jsontsconfig.json
💤 Files with no reviewable changes (18)
- .eslintignore
- src/@types/do-not-zip/index.d.ts
- .huskyrc
- src/lib/getBufferHash.ts
- src/@types/imagesize/index.d.ts
- .prettierrc.json
- .github/workflows/lint.yml
- src/lib/stream-to-buffer.ts
- jest.config.js
- renovate.json
- tsconfig.all.json
- src/lib/yazul-promisified.ts
- .github/workflows/test.yml
- lint-staged.config.js
- .sonarcloud.properties
- .github/workflows/npmpublish.yml
- .eslintrc
- src/lib/signManifest-forge.ts
…IBUTING)
Closes three misses from the original modernization plan that weren't
addressed in the main v7 commit chain.
1. hk.pkl: the Git 2.53 native hook config the plan described. Binds
pre-commit to oxfmt + oxlint (auto-fix) and pre-push to `npm test`.
Validated with `hk validate`. CLAUDE.md already documented `hk install`
as the one-time setup; previously that left contributors with no-op
hooks because the config file itself wasn't committed.
2. WWDR cert expiry runtime warning in src/lib/sign-manifest.ts. If the
bundled WWDR cert is within 90 days of expiry (or already expired),
emit a process warning at module load time. Uses `process.emitWarning`
with distinct codes `WALLETPASS_WWDR_EXPIRED` /
`WALLETPASS_WWDR_EXPIRING`, which integrates with Node's native
warning machinery:
- prints to stderr in the standard `(pid:xxx) [CODE] Name: msg` format
- suppressible via `node --disable-warning=WalletPassWWDRExpiring`
- interceptable via `process.on('warning', ...)`
This is the guard that should prevent another 2023-style silent
expiry. The 2013-2023 G1 cert expired in production for months
before anyone noticed — the whole v7 revival effort started because
of that.
3. CONTRIBUTING.md: documents the Conventional Commits rules (required
for release-please to work correctly), the three quality gates, the
hk setup for local hooks, and explicit scope guidance (no new runtime
deps, no filesystem-required features, Wallet Orders out of scope).
CLAUDE.md is for AI assistants; CONTRIBUTING.md is for humans.
Deliberately did NOT add a `postinstall` script to package.json — that
would pollute every consumer's `npm install` with hint text for a
maintainer-only tool. The hk install instructions are in CLAUDE.md and
CONTRIBUTING.md where they belong.
Remaining gaps from the plan, deferred to v7.1+:
- zip.ts fuzz tests (branch coverage)
- UpcomingPassInformationEntry + secondaryLogo schema
- oxlint --type-check flag (tsgo build step already catches this)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd6753e9af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/lib/sign-manifest.ts (1)
38-67:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse a base64 WWDR literal instead of PEM armor (still unresolved).
The bundled WWDR is still stored as PEM text. This file’s rule requires an inlined base64 payload constant and local decode before parsing.
Suggested minimal refactor
-function parsePkiCertificate(pem: string): PkiCertificate { - const der = new X509Certificate(pem).raw; +function parsePkiCertificate(cert: string | Buffer): PkiCertificate { + const der = new X509Certificate(cert).raw; const ab = der.buffer.slice(der.byteOffset, der.byteOffset + der.byteLength); const asn1 = asn1js.fromBER(ab); if (asn1.offset === -1) { throw new Error('Failed to parse X.509 certificate: invalid ASN.1'); } return new PkiCertificate({ schema: asn1.result }); } -const APPLE_WWDR_G4_PEM = `-----BEGIN CERTIFICATE----- -... ------END CERTIFICATE-----`; +const APPLE_WWDR_G4_B64 = `MIIEVTCCAz2gAwIBAgIUE9x3lVJx5T3G...`; // Override via env for dev/test only. -const APPLE_WWDR_CA_PEM = - process.env['APPLE_WWDR_CERT_PEM'] || APPLE_WWDR_G4_PEM; +const APPLE_WWDR_CA_SOURCE = + process.env['APPLE_WWDR_CERT_PEM'] ?? + Buffer.from(APPLE_WWDR_G4_B64.replace(/\s+/g, ''), 'base64'); -const APPLE_WWDR_CA = parsePkiCertificate(APPLE_WWDR_CA_PEM); +const APPLE_WWDR_CA = parsePkiCertificate(APPLE_WWDR_CA_SOURCE);As per coding guidelines,
src/lib/sign-manifest.ts: "The WWDR cert should be inlined as a base64 string in sign-manifest.ts, not read from keys/wwdr.pem at runtime".🤖 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 `@src/lib/sign-manifest.ts` around lines 38 - 67, The APPLE_WWDR_G4_PEM constant currently inlines the WWDR as PEM armor; replace it with an inlined base64 payload constant (e.g. APPLE_WWDR_G4_B64) and decode it locally before any certificate parsing (use Buffer.from(APPLE_WWDR_G4_B64, 'base64') and convert to PEM if your parser expects PEM), update APPLE_WWDR_CA_PEM to accept an env override that may be base64 (decode process.env['APPLE_WWDR_CERT_PEM'] if present) or fall back to the decoded inlined base64; change references in sign-manifest.ts from APPLE_WWDR_G4_PEM/APPLE_WWDR_CA_PEM to the decoded buffer/string so downstream functions that parse the certificate receive the correct format.
🤖 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.
Duplicate comments:
In `@src/lib/sign-manifest.ts`:
- Around line 38-67: The APPLE_WWDR_G4_PEM constant currently inlines the WWDR
as PEM armor; replace it with an inlined base64 payload constant (e.g.
APPLE_WWDR_G4_B64) and decode it locally before any certificate parsing (use
Buffer.from(APPLE_WWDR_G4_B64, 'base64') and convert to PEM if your parser
expects PEM), update APPLE_WWDR_CA_PEM to accept an env override that may be
base64 (decode process.env['APPLE_WWDR_CERT_PEM'] if present) or fall back to
the decoded inlined base64; change references in sign-manifest.ts from
APPLE_WWDR_G4_PEM/APPLE_WWDR_CA_PEM to the decoded buffer/string so downstream
functions that parse the certificate receive the correct format.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7d7faaf-4f2d-4d85-8a76-a2c68dfede2c
⛔ Files ignored due to path filters (1)
hk.pklis excluded by!**/*.pkl
📒 Files selected for processing (2)
CONTRIBUTING.mdsrc/lib/sign-manifest.ts
✅ Files skipped from review due to trivial changes (1)
- CONTRIBUTING.md
Works through reviewer comments from chatgpt-codex and coderabbit on PR #658. P1 correctness bugs: - src/lib/zip.ts: readZip().getBuffer() now takes compressedSize from the central directory instead of the local file header. Streaming writers (general-purpose bit 3) store 0 in the local header and only populate the central directory. Standards-compliant .pkpass files from such writers would have failed to extract. Added compressedSize to ZipReadEntry and validated it against MAX_ENTRY_SIZE. Major correctness bugs: - src/lib/pass-structure.ts: `set style` now clears stale this.fields.nfc when switching away from storeCard. Previously, a storeCard→coupon transition left the NFC dictionary dangling and serialized an invalid pass. - src/lib/w3cdate.ts: getDateFromW3CString now accepts everything isValidW3CDateString accepts — optional seconds and either `Z` or `±HH:MM` timezone. Previously the validator would green-light `2026-06-01T12:34:56Z` but the parser would crash on it. - src/lib/zip.ts: writeZip now runs the same path-safety check that readZip has always run (no leading slash, no backslash, no `..` segment). Keeps write/read in lockstep and prevents the library from emitting zip-slip bundles. - src/template.ts: fromBuffer duplicate-pass.json guard was checking template.style, which is always undefined after the fromBuffer rewrite. Added an explicit foundPassJson flag. Also throw if the archive has zero pass.json entries instead of silently returning an empty Template. - src/template.ts: pushUpdates no longer throws from inside the .once('connect') callback — that bypasses the surrounding Promise and surfaces as an uncaught exception instead of rejecting pushUpdates(). Replaced with a call to reject + early return. Minor: - src/lib/images.ts: thumbnail error messages reported 90 * densityMulti as the limit but the conditions checked 120/150. Fixed to match. Also corrected "no large than" → "no larger than". - src/lib/localizations.ts: unescapeString now preserves literal backslash-n sequences via sentinel-swap. Previous implementation split on "\\n" before unescaping `\\`, collapsing valid content. - src/lib/semantic-tags.ts: normalizeSemanticValue detects cyclic input and throws instead of recursing until RangeError. - __tests__/template.ts: `push updates` skip predicate now also checks APPLE_PASS_PRIVATE_KEY. - __tests__/w3cdate.ts: dropped tautological `date.getTimezoneOffset() === date.getTimezoneOffset()`. Un-skipped the round-trip test; expanded to three cases (Z timezone, offset, invalid rejection) validating the parser widening. - package.json + ci.yml + CLAUDE.md: double-quoted globs for Windows cmd.exe compatibility. - hk.pkl: use `npx oxlint` / `npx oxfmt` so hook steps find the binaries via node_modules/.bin rather than system PATH. Skipped as false positives / low-ROI (see PR review triage): - Pin-GH-Actions-to-SHA — maintainer policy. - zip.ts EOCD comment-length validation — defensive nicety for archives with trailing comments, which pkpass never uses. - sign-manifest.ts inline-base64 suggestion — reviewer misread the rule; PEM armor is what X509Certificate parses. Verification: - npm run build: clean - oxlint --type-aware: 0/0 - oxfmt --check: all 33 files clean - npm test: 69 tests, 68 pass, 1 skip (live-APNs opt-in) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses P2 finding on PR #658 from chatgpt-codex. signManifest.ts hard-codes rsaEncryption as the CMS signature algorithm because Apple's Pass Type ID certificates are always issued as RSA. If a caller accidentally loaded an EC or Ed25519 private key, the signature bytes wouldn't match the declared algorithm and Wallet would silently reject the pass at open time. Now `setPrivateKey` fails loud at load time with a clear TypeError if `asymmetricKeyType !== 'rsa'`, so the failure happens where it can be fixed (during pass generation) rather than on the device. Added a targeted test that generates a prime256v1 EC key via `node:crypto.generateKeyPairSync` and asserts `setPrivateKey` rejects it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New \`coverage\` job in ci.yml runs tests once on ubuntu-latest/Node 24 with the dual-reporter pattern: - \`spec\` → stdout (humans) - \`lcov\` → coverage/lcov.info (Codecov) Coverage is host/version-independent for a pure-JS library so running it six times would be wasted CI minutes + six confusingly duplicated Codecov uploads per PR. Uses \`codecov/codecov-action@v6\` with \`use_oidc: true\`. Requires the job-level \`id-token: write\` permission; no \`CODECOV_TOKEN\` secret. Codecov server-side verifies the GitHub OIDC issuer/subject claims. \`--test-coverage-exclude\` trims \`__tests__/\**\` and \`dist/**/*.d.ts\` at the reporter level so the lcov output only covers src/ as compiled into dist/*.js. .codecov.yml rewritten: - Old \`parsers.javascript.enable_partials\` was a Jest-era JSON parser option; meaningless for lcov. Dropped. - Added \`fixes\` to rewrite \`dist/lib/foo.js\` → \`src/lib/foo.ts\` so Codecov annotates the actual source files on PRs rather than the emitted dist. tsgo emits source maps (\`sourceMap: true\` in tsconfig.json) which confirms the mapping is accurate. - Status: auto project target (allow coverage to drop 2%), 80% patch target (5% threshold). Tunable once we establish the v7 baseline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lib/w3cdate.ts`:
- Around line 45-50: The validator/parser contract is broken: update
isValidW3CDateString so its regex matches the parser's regex exactly by moving
the end-anchor ($) outside the timezone alternation, changing the
timezone-minute character class from [03]0 to [0-5]\d to allow any 00–59 minute
offset, and use a non-capturing group for the timezone alternation so the
validator accepts the same strings that the parser (the regex in exec within
this file) will parse; ensure the final regex is anchored at both ends and
mirrors the named-capture parser pattern.
🪄 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
Run ID: 8e7d1dd7-de6b-42e2-9e5d-a45ca78827f4
⛔ Files ignored due to path filters (1)
hk.pklis excluded by!**/*.pkl
📒 Files selected for processing (12)
.github/workflows/ci.ymlCLAUDE.md__tests__/template.ts__tests__/w3cdate.tspackage.jsonsrc/lib/images.tssrc/lib/localizations.tssrc/lib/pass-structure.tssrc/lib/semantic-tags.tssrc/lib/w3cdate.tssrc/lib/zip.tssrc/template.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/lib/semantic-tags.ts
- src/lib/images.ts
- src/lib/zip.ts
- .github/workflows/ci.yml
- src/lib/pass-structure.ts
- package.json
- tests/template.ts
Codecov rejected the previous upload with "Unusable report due to issues such as source code unavailability, path mismatch, empty report, or incorrect data format." Root cause: lcov.info reported dist/*.js paths, but dist is gitignored so Codecov couldn't correlate those paths with anything in the committed source tree. The .codecov.yml `fixes` rewrites I added were based on the wrong syntax and weren't firing anyway. Proper fix: add `--enable-source-maps` to the node invocation. tsgo emits .js.map files (sourceMap: true in tsconfig.json); with source maps enabled, node:test's lcov reporter resolves every SF: entry to src/*.ts instead of dist/*.js. No path rewrite needed, no Codecov config workaround. Also added the flag to npm test and CLAUDE.md so local test runs see identical behaviour (stack traces resolve to .ts lines rather than emitted .js). Removed the `fixes` block from .codecov.yml — the paths now match reality on both sides. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/ci.yml:
- Around line 16-17: CI matrix currently uses the floating "24.x" which doesn't
guarantee testing the declared minimum Node version (engines.node: >=24.12.0);
update every occurrence of the Node matrix entry named "node" (the matrix key
with values array used in the workflow) to include the explicit floor version
"24.12.0" instead of "24.x" (i.e., replace "24.x" with "24.12.0" in all
occurrences so the workflow tests the minimum Node 24 runtime as well as 26.x).
🪄 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
Run ID: 876c9a98-55df-4bda-a6a8-1fa8e755e087
📒 Files selected for processing (2)
.codecov.yml.github/workflows/ci.yml
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
There was a problem hiding this comment.
💡 Codex Review
Line 10 in 11398cb
isValidW3CDateString() is missing an end anchor for the Z branch, so values like 2026-01-01T12:00Zjunk are accepted as valid. Callers such as expirationDate/relevantDate setters rely on this validator and will persist these malformed strings into pass.json, producing invalid pass metadata that can fail downstream Wallet validation. Add a final $ that applies to both timezone alternatives.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
grammar pass
Badges:
- Keep only npm version and Codecov coverage. Removed snyk (project
unmaintained upstream), sonarcloud (account defunct), jest badge
(we're on node:test now), packagephobia (we're ESM only, install
size badge misleads).
- Codecov badge URL corrected to new repo path
(tinovyatkin/pass-js, not walletpass/pass-js where the badge 404'd).
Financial Contributors / OpenCollective footer removed. GitHub's
native Sponsors link in .github/FUNDING.yml is the canonical channel
now; no need for a duplicated in-README callout.
Grammar + style:
- "pushing updates" → "push updates"
- "one certificate per Pass Type ID" — tightened sentence
- "JSON specification, structure fields … are represented as arrays,
but items must have distinct key properties. Le sigh." → dropped the
editorial aside; kept the explanation of why the library exposes
a Map-like API.
- Replaced `require(...)` with ESM `import` statements throughout
(package is ESM-only in v7).
- Fixed "or path to logo.pngfile" typo.
- Apple docs link formatting.
- Tightened the "Stay in touch" section.
Content updates for v7:
- Example dates bumped from 2020 to 2026 so readers don't see
long-expired values.
- S3 example uses @aws-sdk/client-s3 (v3) rather than the deprecated
v2 SDK.
- File-write example uses `node:fs/promises` writeFile rather than
`require('fs').writeFile`.
- Reference `constants.PASS_MIME_TYPE` the direct way now that
constants is a plain named export.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📊 Source Code Metrics (this PR vs
|
| File | Cyclomatic | Cognitive | Functions | LLOC | MI |
|---|---|---|---|---|---|
| src/lib/base-pass.ts | 131 (master: 120) 🔴 | 117 (master: 112) 🔴 | 57 (master: 49) 🔴 | 291 (master: 267) 🔴 | 0 ⚪ |
| src/lib/pass-structure.ts | 52 (master: 54) 🟢 | 61 (master: 73) 🟢 | 13 (master: 11) 🔴 | 99 (master: 104) 🟢 | 19.24 (master: 17.53) 🟢 |
| src/lib/localizations.ts | 32 (master: 31) 🔴 | 20 ⚪ | 12 (master: 11) 🔴 | 58 (master: 57) 🔴 | 19.92 (master: 21.09) 🔴 |
| src/template.ts | 57 (master: 60) 🟢 | 83 (master: 87) 🟢 | 9 ⚪ | 133 (master: 124) 🔴 | 9.21 (master: 7.41) 🟢 |
| src/lib/images.ts | 59 (master: 58) 🔴 | 79 (master: 65) 🔴 | 8 ⚪ | 103 (master: 96) 🔴 | 10.84 (master: 11.00) 🔴 |
| src/lib/pass-color.ts | 29 (master: 28) 🔴 | 24 (master: 23) 🔴 | 5 ⚪ | 51 ⚪ | 27.77 (master: 28.29) 🔴 |
| src/lib/fieldsMap.ts | 22 (master: 21) 🔴 | 20 (master: 18) 🔴 | 4 ⚪ | 45 (master: 43) 🔴 | 26.94 (master: 27.18) 🔴 |
| src/lib/w3cdate.ts | 26 (master: 14) 🔴 | 20 (master: 10) 🔴 | 4 (master: 3) 🔴 | 36 (master: 21) 🔴 | 26.64 (master: 30.96) 🔴 |
| src/lib/zip.ts | 39 🆕 | 47 🆕 | 4 🆕 | 129 🆕 | 12.97 🆕 |
| src/lib/nfc-fields.ts | 13 ⚪ | 8 (master: 7) 🔴 | 3 ⚪ | 28 (master: 22) 🔴 | 38.89 (master: 34.29) 🟢 |
| src/pass.ts | 14 ⚪ | 11 ⚪ | 3 ⚪ | 43 (master: 40) 🔴 | 30.55 (master: 28.45) 🟢 |
| src/lib/png-size.ts | 15 🆕 | 20 🆕 | 2 🆕 | 39 🆕 | 31.25 🆕 |
| src/lib/semantic-tags.ts | 12 🆕 | 17 🆕 | 2 🆕 | 34 🆕 | 39.59 🆕 |
| src/lib/sign-manifest.ts | 8 🆕 | 7 🆕 | 2 🆕 | 20 🆕 | 22.94 🆕 |
| src/lib/get-buffer-hash.ts | 2 🆕 | 0 🆕 | 1 🆕 | 4 🆕 | 60.59 🆕 |
| src/lib/get-geo-point.ts | 13 ⚪ | 14 ⚪ | 1 ⚪ | 16 ⚪ | 38.14 (master: 38.32) 🔴 |
| src/lib/normalize-locale.ts | 5 ⚪ | 3 ⚪ | 1 ⚪ | 9 ⚪ | 51.69 (master: 52.20) 🔴 |
| src/@types/do-not-zip/index.d.ts | 0 (was: 1) 🟢 | 0 (was: 0) ⚪ | 0 (was: 0) ⚪ | 0 (was: 1) 🟢 | 0 (was: 97.76) 🔴 |
| src/@types/imagesize/index.d.ts | 0 (was: 1) 🟢 | 0 (was: 0) ⚪ | 0 (was: 0) ⚪ | 0 (was: 1) 🟢 | 0 (was: 97.76) 🔴 |
| src/constants.ts | 1 ⚪ | 0 ⚪ | 0 ⚪ | 15 (master: 16) 🟢 | 21.39 (master: 22.08) 🔴 |
| src/index.ts | 1 ⚪ | 0 ⚪ | 0 ⚪ | 5 (master: 4) 🔴 | 62.99 (master: 71.10) 🔴 |
| src/interfaces.ts | 22 (master: 19) 🔴 | 0 ⚪ | 0 ⚪ | 36 (master: 31) 🔴 | 12.75 (master: 15.03) 🔴 |
| src/lib/getBufferHash.ts | 0 (was: 2) 🟢 | 0 (was: 0) ⚪ | 0 (was: 1) 🟢 | 0 (was: 6) 🟢 | 0 (was: 58.27) 🔴 |
| src/lib/signManifest-forge.ts | 0 (was: 3) 🟢 | 0 (was: 1) 🟢 | 0 (was: 1) 🟢 | 0 (was: 10) 🟢 | 0 (was: 36.22) 🔴 |
| src/lib/stream-to-buffer.ts | 0 (was: 3) 🟢 | 0 (was: 1) 🟢 | 0 (was: 1) 🟢 | 0 (was: 7) 🟢 | 0 (was: 56.25) 🔴 |
| src/lib/yazul-promisified.ts | 0 (was: 4) 🟢 | 0 (was: 0) ⚪ | 0 (was: 2) 🟢 | 0 (was: 14) 🟢 | 0 (was: 40.62) 🔴 |
| src/types.d.ts | 1 🆕 | 0 🆕 | 0 🆕 | 2 🆕 | 70.33 🆕 |
Generated by mehen — the code quality watcher.
Audited CLAUDE.md against current state — scored B (84/100). All additions are genuine maintenance aids, not filler. Additions: - Link to CONTRIBUTING.md in the header (it exists now but wasn't referenced). - "Single test" and "Pre-push check" entries in the command table. - 🔓 marker in the architecture tree for files re-exported from index.ts, so a reader fixing a typo can see at a glance which changes are breaking vs internal. - WWDR rotation instructions upgraded from hand-wavy to a numbered checklist with exact openssl commands and the SHA-256 fingerprint update step that was missing. - New bullet: Apple Pass Type ID signing cert rotation (different from WWDR; expires every 12 months; the maintainer's recurring chore). Previously undocumented. - Note about the WALLETPASS_WWDR_EXPIRING / _EXPIRED process warnings shipped in commit cd6753e — so a future maintainer seeing those codes in a bug report knows where to look. - Note about LF-only line endings enforced by .gitattributes (pointing at the f68a795 fix, which explains WHY). - Explicit Conventional Commits breaking-change syntax (`feat!:` or `BREAKING CHANGE:` footer). Prose "breaking change" doesn't trigger release-please; easy to forget. - New "Debugging a red CI build" section with the gh-CLI commands and the Codecov source-map gotcha we just fought through. .gitignore: added .claude.local.md and .claude/ — personal AI-assistant notes that shouldn't be shared. AGENTS.md unchanged (correctly minimal pointer). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`imagesize@1.0.0` was created 2013-05-06, last modified 2022 (and only
to touch the repository field). It still uses `new Buffer()` which has
been deprecated since Node 10, and ships 305 LOC of JPEG/GIF/PNG
dimension parsing via a stream state machine. Apple pkpass only accepts
PNG, so 90% of that code was unreachable.
New module: src/lib/png-size.ts. ~30 LOC of logic, PNG-spec direct:
0..7 8-byte magic: 89 50 4E 47 0D 0A 1A 0A
8..11 IHDR chunk length
12..15 'IHDR'
16..19 width (big-endian uint32)
20..23 height (big-endian uint32)
Exports two APIs:
- `readPngDimensions(buf: Buffer): { width, height }` — sync, takes a
Buffer with at least 24 bytes, throws TypeError with descriptive
messages for short buffers, bad magic, misaligned IHDR, or zero
dimensions.
- `readPngDimensionsFromFile(path): Promise<{ width, height }>` —
reads only the first 32 bytes from disk (highWaterMark=32, end=31)
so large assets aren't paged into memory for a dimension check.
Drop-ins in src/lib/images.ts:
- `imageSize(stream)` promisified callback → `readPngDimensionsFromFile`
- `imagesize.Parser()` state machine → `readPngDimensions(buf)`
- The old `format: 'gif' | 'png' | 'jpeg'` check is gone because the
new reader only accepts PNG in the first place; checkImage no
longer needs the format field.
- `disableImageCheck: true` path is now faster — we skip PNG parsing
entirely instead of calling imagesize just to throw away the result.
Removed:
- `imagesize` from package.json deps (1 less package on `npm ci`)
- `declare module 'imagesize'` + its 28-line ambient typing block in
src/types.d.ts (only color-name declaration remains, much smaller
surface)
Verification:
- 78 tests pass, 1 skip (live-APNs opt-in).
- New __tests__/png-size.ts adds 8 targeted tests: real fixture,
round-trip between file/buffer APIs, short buffer, wrong magic,
misaligned IHDR, zero dimensions, synthesized 1x1 header,
non-PNG file.
- Bundle smoke: `grep -c imagesize /tmp/bundle.mjs` = 0.
- npm audit --production: no change (was already clean).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
oxlint (preserve-caught-error) flagged the two catch blocks around the
new PNG reader. Re-throw with `{ cause: err }` so the stack for the
underlying parse failure survives. Zero-cost for callers who don't
inspect .cause.
There was a problem hiding this comment.
💡 Codex Review
Line 10 in c2dded7
The isValidW3CDateString regex currently accepts malformed values (for example impossible dates like 2026-19-39...) and also accepts trailing garbage for ...Z timestamps because the end anchor is only applied to the offset branch. Since getW3CDateString returns any string that passes this validator unchanged, malformed timestamps can be emitted into pass.json instead of being normalized or rejected, leading to invalid pass metadata at runtime.
Lines 462 to 466 in c2dded7
The maxDistance setter documents and error-messages this field as a positive integer, but the implementation only checks Number.isInteger(v). As a result, values like -100 are accepted and serialized, which produces semantically invalid relevance-distance data instead of failing fast for callers.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two review findings from PR #658, both real bugs. ### w3cdate.ts isValidW3CDateString — validator accepted bad input Two separate bugs in the same regex: 1. The `$` end-anchor was inside the timezone alternation: `(Z|([+-][01]\d:[03]0)$)`. Only the offset branch was anchored to end-of-string; the `Z` branch was not. So `"2026-01-01T12:34Zgarbage"` validated as a "valid W3C date," then crashed the parser. 2. Timezone-minute class was `[03]0`, accepting only `:00` and `:30` offsets. :45 offsets exist in the real world — Nepal (+05:45), Chatham Islands (+12:45), and historic zones. :15 too. Fixed to `[0-5]\d` to accept any valid minute. Also cleaned up the alternation to non-capturing groups (`(?:...)`) since no one reads the capture groups. Added two regression tests covering trailing-garbage rejection and :30/:45 offset acceptance. ### template.ts Template.fromBuffer — notpass.json misidentification The pass.json detection regex `/\/?pass\.json$/i` used an optional leading slash. Any filename ending in `pass.json` matched — including `notpass.json` at root and `foo/notpass.json` in a subdirectory. Impact: an archive containing both a real `pass.json` and a `notpass.json` would either (a) parse the decoy as pass metadata, or (b) throw the "more than one pass.json" error when only one real file exists. Anchoring to `(?:^|\/)pass\.json$` restricts the match to a full path segment. Added a test that synthesizes a two-entry ZIP via our own `writeZip` and verifies Template.fromBuffer picks the right one. Tests: 78 → 81, all passing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9c59c01f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two review findings from PR #658, both real bugs with concrete repros. ### semantic-tags.ts: shared subtrees mistaken for cycles The normalizer tracked every previously-seen object in a WeakSet, so a valid acyclic input like \`{ a: shared, b: shared }\` (common in JS — one venue location object referenced twice) threw "Semantic tags must not contain cyclic references". Fix: track only the nodes currently on the active recursion path, pop them on the way out. Genuine cycles still throw; shared subtrees now serialize normally. ### base-pass.ts toJSON: Date values inside relevantDates toJSON only normalized Date values at the top level, so Date objects inside RelevantDateEntry (startDate/endDate/relevantDate) fell through to the default JSON.stringify path, which calls Date.prototype.toJSON and emits "2026-06-01T12:00:00.000Z" (milliseconds + trailing Z) — diverging from the W3C format (YYYY-MM-DDTHH:MM±HH:MM) that every other date field uses. Inconsistent timestamps within the same pass. Fix: replace the inline loop with \`normalizeDatesDeep\`, a tiny recursive walker in w3cdate.ts. It descends into arrays and plain objects (\`value.constructor === Object\`) and converts every Date it finds. Class instances (PassColor, FieldsMap, NFCField, …) are NOT descended into — they carry their own toJSON which already does the right thing for their internal state. Avoids the fragility of the hardcoded \`field === 'relevantDates'\` special case from an earlier draft: any future nested-Date schema field (UpcomingPassInformation, PKPassBookMarkup, …) is handled without code changes. \`formatVersion: 1\` still seeds the output first (spread preserves insertion order), matching the previous output byte-for-byte for non-Date fields. Tests: 81 → 84. - semantic: shared subtree round-trips; genuine cycle still throws - base-pass: relevantDates Date entries render as W3C strings without milliseconds, pre-formatted strings pass through unchanged Verified byte-identical non-Date serialization: \`{"formatVersion":1,"description":"test","serialNumber":"s"}\` Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
pass-js/src/lib/localizations.ts
Line 39 in 95df90a
Using a literal U+0000 character for SENTINEL makes src/lib/localizations.ts a binary file from Git’s perspective, which breaks normal text diff/review behavior and can cause text tooling (formatters, linters, patch tools) to skip or mishandle this source file; it also makes unescapeString collide with real NUL data because any genuine \u0000 in input will be rewritten as a backslash in the final replacement step. Use an escaped sentinel representation (for example '\\0' or another printable token) to avoid binary-text and collision issues.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
Revive
@walletpass/pass-jsafter a 5-year dormancy. Ships as v7.0.0 with a modern ESM-only build, current Apple WWDR cert, every worthwhile community PR applied, and a release-automation setup that should prevent this class of neglect from recurring.Headline changes:
notAfter=2023-02-07, every downstream user has been silently shipping invalid passes) to G4 (valid through 2030-12-10).node-forgeentirely (had 6 CVEs disclosed late 2025 / early 2026). PKCS#7 SignedData now built withpkijs+ nativenode:crypto. Round-trip verified viaopenssl cms -verify.__dirname, ship-ready for AWS Lambda / Vercel / CF Workers.tsgo), oxlint, oxfmt,node:testreplacing Jest.hkfor git hooks.appLaunchURL, NFCrequiresAuthentication,relevantDates,preferredStyleSchemes,disableImageCheck.Commits (logical grouping)
Foundation
050d4a4feat!: modernize to TS 7 preview / oxlint / oxfmt / node:test / Node 24.12+— the big rewrite. Engines>=24.12.0,"type": "module","exports"map,"sideEffects": false. Drops 6 runtime deps (node-forge,yauzl,do-not-zip,event-iterator,buffer-crc32) by inlining a ~200-LOC ZIP reader+writer atsrc/lib/zip.ts. Crypto rewrite insrc/lib/sign-manifest.tswith G4 inlined as a base64 string so bundlers pick it up automatically.1769073fix: address CodeRabbit P0/P1 review findings— strict bounds checks in ZIP reader,set styleno longer clobbers existing NFCField, grammar fixes.fe49fb8test: migrate test suite from Jest to node:test— 67 tests, no shim layer, straightnode:assert/strict. Snapshot tests use nativet.assert.snapshot(). Signing tests self-generate throwaway certs (theAPPLE_PASS_*CI secrets have been expired since 2020).d82f013chore: clean up .npmrc typo, lint errors, and formatter config— fixes the long-standingsave-exact§=truetypo in.npmrcthat emitted an npm warning on every invocation.Community PRs merged (all authors credited in commit trailers)
4556236feat(keys): update WWDR cert to G4— closes Update wwdr.pem #616, credits PR Update Apple Worldwide Developer Relations cert #626 by @lucyyyyyyya33fbb3feat: add Options.disableImageCheck— closes Disable checking image dimensions #480, credits PR Add option property disableImageCheck #479 by @akoufa0828097feat: add appLaunchURL + NFC requiresAuthentication— credits PR Missing appLaunchURL mapping #652 by @navelencia25dd5b7feat: add semantic tags support— closes [Feature] Supporting Semantic Tags in Wallet Passes #75, credits PR Add semantic tags support #657 by @apples-kkskAdditive
e5c4d3bfeat: add iOS 18+ schema fields (relevantDates, preferredStyleSchemes)— plus deprecation notices on singularrelevantDateandbarcode.Tooling
233ef96chore(ci): modernize workflows, add AI-tooling docs and code metrics— newci.yml(matrix ubuntu/macos/windows × Node 24+26), bundle smoke test (esbuild + ncc),mehen.ymlfor code-metrics PR comments,dependabot.ymlreplacing the deleted renovate config,CLAUDE.md+AGENTS.mdfor future AI-assisted maintenance.8f324bfci: add release-please + npm trusted publishing— conventional-commit driven release automation. After this PR merges, release-please will open a PR proposing 7.0.0. Merging that tags the release and triggers OIDC publish.Quality gates
npm run build(tsgo)oxlint --type-awareoxfmt --checknpm test.skip)openssl cms -verifypasses on emitted distBreaking changes (v6 → v7)
>=10.10.import(). Norequire('@walletpass/pass-js').Template.certificateshape changed — now a PEM string internally rather than aforge.pki.Certificateobject. Public API (setCertificate,setPrivateKey,loadCertificate) unchanged.Template.keyshape changed — now anode:cryptoKeyObjectrather than aforge.pki.PrivateKey.NFCField.setPublicKey(key)takes PEM only — the previously-undocumentedforge.pki.PublicKeyobject form is gone (it was unusable without node-forge anyway).Template.fromBuffernow synchronous internally (no more callback-style yauzl wrapping). Public API unchanged (awaitstill works).PR closures
All 18 open PRs have been triaged and closed:
Before merging
@walletpass/pass-jspointing attinovyatkin/pass-js+ workflowpublish.yml(in progress)CLAUDE.mdaccuracy__tests__/*.ts.snapshotAfter merging
feat!:commitv7.0.0and triggerspublish.ymlNPM_TOKENsecret from repo settings once trusted publishing is proven.pkpasswith a valid Pass Type ID cert, install on a physical iPhone (iOS 18+), confirm Wallet accepts it🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
Documentation