Add semantic tags support - #657
Closed
apples-kksk wants to merge 2 commits into
Closed
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR implements Apple Wallet semantic tags support by introducing types and runtime normalization for semantic metadata, extending PassBase and FieldsMap to handle semantics at both pass and field levels with automatic Date-to-W3C-string conversion, and providing test coverage and documentation. ChangesSemantic Tags Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/semantic-tags.ts (2)
16-21: Consider adding clarifying comment for null guard.The condition
value && typeof value === 'object'correctly guards againstnull(sincetypeof null === 'object'in JavaScript), but this subtlety might not be immediately obvious to future maintainers.📝 Optional clarifying comment
if (Array.isArray(value)) return value.map(normalizeSemanticValue); + // typeof null === 'object', so check truthiness to exclude null if (value && typeof value === 'object') {🤖 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/semantic-tags.ts` around lines 16 - 21, Add a short clarifying comment above the condition `value && typeof value === 'object'` (used in the `normalizeSemanticValue` flow) that explicitly states this also guards against `null` because `typeof null === 'object'` in JavaScript; keep the comment concise and place it immediately before the `if` so future maintainers understand the null check intent.
9-10: ⚡ Quick winImprove error message clarity.
The error message "Semantic tag dates must be valid Date instances" is misleading because this check occurs after line 8 confirms the value IS a Date instance. The actual issue is that the Date is invalid (has NaN time).
💬 Suggested error message improvement
if (!Number.isFinite(value.getTime())) - throw new TypeError(`Semantic tag dates must be valid Date instances`); + throw new TypeError(`Semantic tag Date values must be valid (not Invalid Date)`);🤖 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/semantic-tags.ts` around lines 9 - 10, The thrown TypeError after checking value.getTime() should clarify that the Date instance is invalid (NaN time) rather than implying it's not a Date; update the error message thrown where value.getTime() is tested (the throw new TypeError(...) in semantic-tags.ts) to something like "Semantic tag date must be a valid Date object (contains NaN time)" or "Semantic tag dates must be valid Date objects (getTime() is NaN)" so the message accurately reflects the failure condition.
🤖 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.
Nitpick comments:
In `@src/lib/semantic-tags.ts`:
- Around line 16-21: Add a short clarifying comment above the condition `value
&& typeof value === 'object'` (used in the `normalizeSemanticValue` flow) that
explicitly states this also guards against `null` because `typeof null ===
'object'` in JavaScript; keep the comment concise and place it immediately
before the `if` so future maintainers understand the null check intent.
- Around line 9-10: The thrown TypeError after checking value.getTime() should
clarify that the Date instance is invalid (NaN time) rather than implying it's
not a Date; update the error message thrown where value.getTime() is tested (the
throw new TypeError(...) in semantic-tags.ts) to something like "Semantic tag
date must be a valid Date object (contains NaN time)" or "Semantic tag dates
must be valid Date objects (getTime() is NaN)" so the message accurately
reflects the failure condition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e25ed6be-b664-45ed-a675-279148fcc951
📒 Files selected for processing (10)
README.md__tests__/base-pass.ts__tests__/fieldsMap.tssrc/constants.tssrc/index.tssrc/interfaces.tssrc/lib/base-pass.tssrc/lib/fieldsMap.tssrc/lib/pass-structure.tssrc/lib/semantic-tags.ts
Owner
|
Thanks @apples-kksk — your semantic-tags implementation shipped in v7.0.0 as commit Closing as applied. Credit is preserved in the commit body. Really appreciate this one — closes a 6-year-old feature request (#75) and unlocks iOS 18 poster event tickets. |
3 tasks
tinovyatkin
added a commit
that referenced
this pull request
May 9, 2026
* feat!: modernize to TS 7 preview / oxlint / oxfmt / node:test / Node 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> * fix: address CodeRabbit P0/P1 review findings 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> * test: migrate test suite from Jest to node:test 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> * chore: clean up .npmrc typo, lint errors, and formatter config - .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> * feat(keys): update WWDR cert to G4 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> * feat: add Options.disableImageCheck to skip image dimension validation 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> * feat: add appLaunchURL + NFC requiresAuthentication 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> * feat: add semantic tags support 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> * feat: add iOS 18+ schema fields (relevantDates, preferredStyleSchemes) 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> * chore(ci): modernize workflows, add AI-tooling docs and code metrics 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> * ci: add release-please + npm trusted publishing 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> * ci: fix Windows line-endings + pin mehen to v0 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> * fix(localizations): use literal \n instead of os.EOL on .strings escapes 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> * ci: fix bundle-smoke + make mehen non-blocking - 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> * ci(bundle-smoke): inject createRequire shim for bundled CJS transitives 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> * feat: gap-closure items from the v7 plan (hk.pkl, WWDR warning, CONTRIBUTING) 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> * fix: address PR review findings (P1 zip bug + 9 other actionable items) 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> * fix(template): reject non-RSA private keys in setPrivateKey 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> * ci: upload coverage to Codecov via OIDC 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> * fix(ci): use --enable-source-maps so Codecov sees src/*.ts paths 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> * docs: clean up README — remove old badges, drop Financial Contributors, 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> * docs(CLAUDE.md): close gaps found during doc audit 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> * feat: replace `imagesize` npm dep with a 30-LOC in-repo PNG reader `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> * fix(images): preserve original error via { cause } in PNG validation 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. * fix: validator/parser mismatches in W3C date + pass.json regex 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> * fix: recursive Date normalization + per-path cycle detection 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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
|
Congrats on shipping v7.0.0, and thanks for carrying this into the release. Glad the semantic tags work helped close #75. The commit reference is also helpful for tracking the bounty side. |
Author
|
Hi @tinovyatkin, sorry for another ping here. I'm posting on this PR because this is where you confirmed that #657 was applied in v7.0.0 via commit I may be missing something about the IssueHunt flow, but the IssueHunt page for #75 still appears to show the bounty as funded/ready rather than rewarded. Since this may be time-sensitive for the Friday payout cycle, would you mind checking whether you see any maintainer-side approve/reward action for this PR on IssueHunt? If there is no such action available from your account, no worries at all. I'll follow up with IssueHunt support instead. Thanks again for shipping this in v7.0.0 and for preserving the credit. |
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
3 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.
Closes #75.
Adds Apple Wallet semantic tag support for generated passes. Semantic tags can now be set on the root pass dictionary with
pass.semanticsor on individual pass fields withsemantics, matching Apple Wallet's pass.json placement options.Changes:
SemanticTags/SemanticTagValueTypeScript types and exports them from the package entrypoint.semanticswhen constructing or loading passes.semanticsthroughFieldsMap.Datevalues in semantic tags to the existing W3C date string format.Verification:
Notes:
npm ciis currently blocked becausepackage.jsonandpackage-lock.jsonare out of sync in the base branch, so I avoided changing the lockfile.@destinationstransfers/eslint-plugin/eslintpackage export incompatibility.Summary by CodeRabbit
New Features
Documentation
Tests