HF-329: re-port the license key reader to the entitlement key format (7/8) - #1740
HF-329: re-port the license key reader to the entitlement key format (7/8)#1740marcin-kordas-hoc wants to merge 2 commits into
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
hyperformula-docs | a425d37 | Commit Preview URL Branch Preview URL |
Aug 26 2026, 03:39 AM |
Performance comparison of head (a425d37) vs base (3dbcbe6) |
9933ba3 to
4ab3cc6
Compare
3ad0a5b to
523316e
Compare
Tobiadefami
left a comment
There was a problem hiding this comment.
Reviewed at 523316e together with the paired tests at 81f180ce. I checked format routing, checksum-before-parse ordering, the strict-shape/lenient-vocabulary contract, per-product expiry, the invalid-key invariant, hostile-payload handling, provenance, docs and changelog, current automated findings, and later stacked branches. The 15 focused license suites pass (256 tests), and all current engine checks are green. I found one material shape-validation issue: a single-element array is accepted where an entitlement date must be a YYYY-MM-DD string, as noted inline.
| // The cast mirrors upstream, where the call is untyped: `parseIsoDate` stringifies its | ||
| // argument before matching it against the `YYYY-MM-DD` shape, so a non-string value is | ||
| // rejected by the shape check rather than by a type guard here. | ||
| parseIsoDate(value as string, 'license') |
There was a problem hiding this comment.
value as string is only a compile-time assertion, while parseIsoDate stringifies at runtime. A checksum-valid payload with usage_until: ["2099-12-31"] therefore passes extraction unchanged and resolveLicense reports it VALID; I reproduced both on this head. That contradicts T7/J6's strict YYYY-MM-DD string shape and makes the declared usage_until?: string type untrue, so malformed input can activate a restricted entitlement instead of taking the invalid-key path. It would help to require typeof value === 'string' before parsing, fix the mirror upstream first, and pin the array case.
Upstream handsontable/license-key 4.0.0 (DEV-2512) deleted src/typed-key/ and replaced the tagged key format with the entitlement key format: <prose>, blank line, [<base64url-payload><sha512-checksum>]. The tagged format was never issued to anyone (its 3.5.0 carrier was never released), so the old reader is removed rather than kept alongside. Re-vendored from src/entitlement-key/ at tag 4.0.0: detect-format and extract-key-data are new ports; sha512 and utils are byte-identical upstream and carry over. The reader is schema-free by upstream design, so default-schema is no longer vendored and TIER_TO_CAPABILITY_TOKEN (the tagged format's tier adapter) is gone with the format that fed it. Resolution reads HyperFormula's own product entry only: capabilities verbatim, exactly one of usage_until/release_until (the reader enforces the shape), notice/grace, flags (trial + the three silent spellings). Legacy 25-character keys and the literals are untouched; the invariant stands - only a VALID entitlement key may restrict the entitlement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pxNP45obT2LZfjitaCv9o
`isIsoDate` asserted `value as string` and handed it to `parseIsoDate`, which stringifies
its argument before matching the `YYYY-MM-DD` shape. The assertion is compile-time only, so
any value whose `String()` spells a date passed the check at runtime. A single-element array
is the realistic case, and it is not hypothetical: `{"usage_until": ["2099-12-31"]}` with a
valid checksum extracted cleanly, stored an `object` in a field declared `string`, and
resolved VALID.
That is the wrong direction to fail in. The payload shape is fatal by contract, so such a key
must take the invalid-key path; instead it activated a RESTRICTED entitlement, which under
our fail-open model means a malformed key grants a customer LESS than a broken one would.
`release_until` had the same hole.
Measured after the change: the array cases on both date fields return `null` from extraction
and resolve invalid, while a string date is untouched.
The mirror of this function lives upstream in `license-key`; the fix belongs there too, and
`PROVENANCE.md` should be revisited once it lands so the vendored copy does not silently
diverge from the source it pins.
Reported by Tobiadefami on this PR.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
523316e to
a425d37
Compare
4ab3cc6 to
3dbcbe6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## spike/hf307-addon-grants #1740 +/- ##
============================================================
- Coverage 97.38% 97.38% -0.01%
============================================================
Files 204 204
Lines 16236 16233 -3
Branches 3493 3482 -11
============================================================
- Hits 15812 15809 -3
Misses 424 424
🚀 New features to boost your workflow:
|
7/8 of the HF-307 stack (#1728 → #1729 → #1730 → #1731 → #1736 → #1737 → this). Implements HF-329 ("Import license key validator shared code to HF"). Pairs with
hyperformula-tests#41— merge the tests PR first (CI clones it by branch name).Why now
Upstream
handsontable/license-key4.0.0 (18.08, DEV-2512) deletedsrc/typed-key/— the exact directory our vendored reader was ported from — and replaced the tagged key format with the entitlement key format:my.handsontable.com already pins 4.0.0 and mints entitlement keys for trials, so the vendored reader was reading a format that no longer exists anywhere. The byte-level rules are specified in the key spec's "Technical implementation" addendum (T1–T14).
What changed
Only the envelope. The entitlement layer — token resolution, the capability table, gate A/B,
ensureCapability— is untouched by design.src/entitlement-key/at tag 4.0.0:detectFormat.tsandextractKeyData.tsare new ports;sha512.ts/utils.tsare byte-identical upstream (same hashes) and carry over.PROVENANCE.mdrecords the new tag, commit, and sha256 table.defaultSchema.tsis no longer vendored; the only name the engine reads is its ownhyperformulaproduct entry.resolveLicenseroutes on the vendoreddetectLicenseKeyFormat(normative T12 order: literals → bracketed block → legacy shape). Literal keys and legacy 25-character keys reachcheckLicenseKeyValiditycompletely unchanged.unrestrictedEntitlement()(mutation-tested in the suite).Decisions to confirm in review
TIER_TO_CAPABILITY_TOKEN— the entitlement format has notierfield.YYYY-MM-DDstrings, never routed throughDateformatting (fixture J11).Testing
Fixtures re-minted with upstream's real generator at 4.0.0 and round-tripped through upstream's extractor; the addendum's T14 conformance vectors pinned 1:1, baselined on its T13 worked example verbatim. Full private suite: 516 suites / 6430 passed, 3 pre-existing skips;
tsc --noEmitand ESLint clean.Found while working on this, NOT fixed here
CHANGELOG.md's[Unreleased]has noRemovedentry for the static function metadata API removed by HF-349 (#1724) — a breaking change currently shipping into 3.5.0 unannounced. Raised separately.🤖 Generated with Claude Code
https://claude.ai/code/session_019pxNP45obT2LZfjitaCv9o
Note
Medium Risk
Rewrites proprietary license parsing and routing, which is business-critical, but legacy/gpl paths and the “bad key → unrestricted entitlement” invariant are preserved; risk is mainly wrong verdicts or grants for new entitlement keys and hostile payloads.
Overview
Replaces the internal tagged (“typed”) proprietary key reader with the upstream entitlement key format — human-readable text plus a trailing
[base64url-payload + sha512]block — vendored fromhandsontable/license-key4.0.0. The old tagged format anddefaultSchema.ts/ tier-to-package mapping are removed; classic 25-character keys andgpl-v3still route through the unchanged legacy validator.resolveLicensenow usesdetectLicenseKeyFormat(literals → bracketed block → legacy shape) andextractEntitlementKeyData. Terms come only from the payload’shyperformulaproduct entry (per-entry dates, notice, grace, flags); keys for other products or with no HyperFormula entry are valid but grant nothing for this library.Behavior fixes on the entitlement path: capability tokens are appended one-by-one so huge payloads cannot crash construction via
push(...array); the “grant all fivefeat:*areas” fallback runs only when the key names a recognizedfeat:*token, so unknownfeat:*tokens stay inert per D3. Console notice identity uses the checksum plus closing bracket (129 trailing chars).Docs and CHANGELOG describe both key formats, validation axes (
usage_untilvsrelease_until), and the valid-key expiry notice.Reviewed by Cursor Bugbot for commit a425d37. Bugbot is set up for automated code reviews on this repo. Configure here.
Spec-to-ship review (2026-08-20): 3 findings fixed here
A six-dimension adversarial review ran over this PR, #1741 and #1743 (spec conformance, decision
conformance, port fidelity, hostile payloads, dialect table, API surface). What it found against
this commit, all fixed above and each one verified by running it:
capabilityTokens.push(...grant.capabilities)spread an attacker-sized array into a call: measured, a checksum-valid key carrying 125 000
tokens threw
RangeError: Maximum call stack size exceededout ofbuildFromArrayinstead ofresolving to a verdict — the one thing this module's own doc comment promises never happens. Now
appended element by element; 200 000 tokens (a 3 MB key) resolves in 141 ms. Pinned by a test.
rejects one character changed in the payload segment [catches: the checksum doing nothing]passed with the checksum comparison deletedoutright, because the flipped character corrupted the payload's
productskey and the T7 shapecheck rejected it first. Mutation-verified in both directions: with the comparison removed only
the checksum-segment vector failed. Replaced with a well-formed payload (the T13 record with
notice: 61) carrying T13's original checksum, plus a positive control asserting the samepayload with its own checksum is accepted — so the rejection can only come from the checksum.
PROVENANCE.md's divergence DATE type is not supported by arithmetic and comparison operators #3 claimed the typedentry's "fields carry their verified types", but the date field is verified only by matching
String(value)againstYYYY-MM-DD, so an array holding one date string passes and the declaredstringis wider than the value (faithful to upstream — the manifest was the thing overstating).And
isNonNegativeIntegerhad drifted to the globalisFinitewhere upstream usesNumber.isFinite: behaviour-identical behind thetypeofguard, but an undocumented edit insidea hash-pinned mirror. Both corrected.
Also corrected in the guide: the classic-key example showed 20 characters next to the sentence
"25 characters", and the entitlement-key paragraph said the text "may be re-wrapped or re-formatted
… without affecting the key" — true of the prose, false of the block, which has to arrive character
for character.