HF-307: license key reader (entitlement key format, rev 6), resolution, and capability table - #1730
marcin-kordas-hoc wants to merge 8 commits into
Conversation
|
Task linked: HF-307 Implement feature packages and add-ons in HF |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
hyperformula-docs | ce18576 | Sep 22 2026, 02:52 PM |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Performance comparison of head (ce18576) vs base (14f1ed5) |
|
bugbot run |
|
bugbot run |
05882e1 to
ab1bba1
Compare
Approval retracted because the repository requires obsolete PR descriptions to be updated. The code and paired tests remain reviewed with no scope-aligned finding; re-approval only requires confirming the refreshed PR description and unchanged head.
Tobiadefami
left a comment
Tobiadefami
left a comment
There was a problem hiding this comment.
Reviewed at ab1bba1 together with the paired tests at 9977ea88. The typed-key reader, license resolution, capability mapping, legacy-key invariant, and current automated findings were checked. The six changed paired suites pass (131 tests), and the current engine checks are green. I found one material issue: the expired-key console date ignores the key's grace period, as noted inline.
…able Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
8dcca66 to
e88b96b
Compare
marcin-kordas-hoc
changed the title
| ### Proprietary license key formats | ||
|
|
||
| Your proprietary license key is in one of two formats, and both work the same way: | ||
|
|
||
| * A classic key: 25 characters in five dash-separated groups, for example | ||
| `1a2b3-4c5d6-7e8f9-0a1b2-3c4d5`. | ||
| * An entitlement key: a short, human-readable license text that ends with a machine-readable | ||
| block in square brackets. Assign the whole text to the `licenseKey` option, or just the | ||
| bracketed block — the block is the only part HyperFormula reads, so both work. The text around | ||
| the block may be re-wrapped on its way to you (for example, by an email client) without | ||
| affecting the key; the block itself has to arrive character for character. | ||
|
|
There was a problem hiding this comment.
Don't mention it in the public docs.
There was a problem hiding this comment.
@sequba Done — the whole "Proprietary license key formats" section is out of the guide.
I also applied the same reasoning to the changelog, since it is the same disclosure in another public file: the entry for this PR described the format in the same words ("a human-readable text ending with a machine-readable block in square brackets"). It now names the change and what is unaffected, without describing the format. Say the word if you would rather have that wording back.
| the block may be re-wrapped on its way to you (for example, by an email client) without | ||
| affecting the key; the block itself has to arrive character for character. | ||
|
|
||
| ### Proprietary license key validation |
There was a problem hiding this comment.
This section should say that the validation mechanism depends on the terms of your contract.
There was a problem hiding this comment.
@sequba Done. The section no longer enumerates the two date axes; it says that which versions a key covers, and for how long, follows from the terms of your contract, and that HyperFormula applies them locally. I kept the existing tip about no internet connection, since that was not what you objected to.
Worth knowing: this section predates HF-307 on develop and this PR had expanded it. The rewrite takes it back past the original wording rather than only undoing the expansion.
| * events, and a page that swaps keys (or a test suite that builds one engine per key) must still | ||
| * warn for the second one even though the first already consumed a shared flag. | ||
| */ | ||
| const _noticedKeys = new Set<string>() |
There was a problem hiding this comment.
Why it is per key? Why do we need to manage more keys in single instance of HF?
There was a problem hiding this comment.
@sequba Small correction to the premise first: _noticedKeys is a module-level const, not instance state. It is not one engine holding several keys — it is several engines on one page load.
The case it exists for is a page that swaps the key, or a suite that builds one engine per key. Two keys approaching their own expiry are two different events, and with a single boolean the second one is silently swallowed because the first consumed it. The flag above it stays a single boolean on purpose, because "a key is invalid" means the same thing whichever key caused it.
If you would rather have one notice per page load regardless, say so and it collapses to a boolean — it is a two-line change. I only wanted the reason on record either way.
| @@ -0,0 +1,100 @@ | |||
| # Vendored entitlement-key reader — provenance and drift control | |||
There was a problem hiding this comment.
Let's rename the directory vendor to handsontable-license-key-parser
There was a problem hiding this comment.
@sequba Done. src/license/vendor/ is now src/license/handsontable-license-key-parser/, moved with git mv so blame and --follow walk through it, and the paired test directory is renamed to match. Only paths changed; the prose about a "vendored reader" stays, since it describes the relationship rather than the location.
| This replaces the earlier port of `src/typed-key/` at `7553d0d1` (2026-08-11). Upstream 4.0.0 | ||
| (DEV-2512) **deleted** that directory and replaced the tagged key format with the entitlement key | ||
| format; the tagged format never reached customers, so the old reader was removed here rather than | ||
| kept alongside. |
| `utils.js` and `sha512.js` are byte-identical between `src/typed-key/` at the old pin and | ||
| `src/entitlement-key/` at `4.0.0` (same hashes as the previous revision of this table), so their | ||
| ports carried over unchanged apart from this file's path references. |
| `allowJs` is off in HyperFormula's `tsconfig.json` and `strict` is on, so these files are a port | ||
| rather than a copy. Beyond adding types, the semantics were kept identical except for the | ||
| following, which a drift review should expect to see: |
There was a problem hiding this comment.
Can we add exceptions to tsconfig so these files could be the byte-identical copies of the upstream?
There was a problem hiding this comment.
@sequba I would like this too, and it is worth more than tidiness: byte-identical files turn drift control from "re-read and re-port the changed file" into sha256sum, which is the whole point of the hash table in that document.
But it is not just allowJs. The files would have to be emitted and bundled as .js inside a strict build that also emits declarations, which means either checkJs: false plus a hand-written .d.ts per file, or @ts-check-clean upstream sources. I have not measured whether the rollup and declaration steps survive that.
Before I spend that: is the goal (a) a mechanical drift check, or (b) literally no local edits? If (a), there is a cheaper route — keep the port and add a CI job that clones the upstream at the pinned commit and compares. §7.1 of the spec asks for a drift check in CI anyway, and the document currently admits the check is manual.
One thing that collides either way: there is a deliberate local divergence in extractKeyData.ts (see my separate comment on the divergence list). A byte-identical copy cannot keep it.
| /** The group tokens each package adds, exactly as the packaging doc's §4 table states them. */ | ||
| const MATH_ENGINE_GROUPS = ['fun:math.a', 'fun:stat.a', 'fun:logic.a', 'fun:operator.a', 'fun:info.a', 'fun:lookup.a'] | ||
| const CALCULATED_FIELDS_GROUPS = ['fun:time.b', 'fun:text.b', 'fun:logic.b', 'fun:math.b', 'fun:stat.b'] | ||
| const SPREADSHEET_GROUPS = [ | ||
| 'fun:lookup.c', 'fun:math.c', 'fun:stat.c', 'fun:time.c', 'fun:text.c', 'fun:info.c', 'fun:logic.c', | ||
| 'fun:finance.c', 'fun:engineer.c', 'fun:array.c', | ||
| ] |
There was a problem hiding this comment.
These groups should not be hardcoded. The code should operate on a capabilities level and should know nothing about what capabilities are in what package.
There was a problem hiding this comment.
@sequba On the group constants you are simply right: MATH_ENGINE_GROUPS / CALCULATED_FIELDS_GROUPS / SPREADSHEET_GROUPS are the packaging table transcribed into the engine — the comment above them literally says "exactly as the packaging doc's §4 table states them". That belongs to whatever mints keys, not here.
Your broader point I want to scope before acting on, because it touches a decision we recorded in August (D8): the HyperFormula token registry belongs to us — rev 6 §2.2, and the shipped 4.0.0 schema declares hyperformula: [] with the whitelist off. Two readings:
- (a) the engine knows token → functions but nothing about package → tokens. Then the three constants and the
functions_1..4grants go, and thefun:<family>.<tier>table stays. - (b) the engine knows nothing, and a key carries function ids directly. That retires the group-token vocabulary and reverses D8.
Which one? I would rather reopen D8 deliberately than by accident. This also gates the unrestricted reshape, since both rewrite the same file.
| [FUN_ALL_TOKEN, {functions: [...functions4Grant.functions], features: []}], | ||
| ...groupEntries, | ||
| ...singleFunctionEntries, | ||
| ]) |
There was a problem hiding this comment.
In this file there are a lot of things that don't belong in HF repo
There was a problem hiding this comment.
@sequba On the group constants you are simply right: MATH_ENGINE_GROUPS / CALCULATED_FIELDS_GROUPS / SPREADSHEET_GROUPS are the packaging table transcribed into the engine — the comment above them literally says "exactly as the packaging doc's §4 table states them". That belongs to whatever mints keys, not here.
Your broader point I want to scope before acting on, because it touches a decision we recorded in August (D8): the HyperFormula token registry belongs to us — rev 6 §2.2, and the shipped 4.0.0 schema declares hyperformula: [] with the whitelist off. Two readings:
- (a) the engine knows token → functions but nothing about package → tokens. Then the three constants and the
functions_1..4grants go, and thefun:<family>.<tier>table stays. - (b) the engine knows nothing, and a key carries function ids directly. That retires the group-token vocabulary and reverses D8.
Which one? I would rather reopen D8 deliberately than by accident. This also gates the unrestricted reshape, since both rewrite the same file.
# Conflicts: # src/license/CapabilityRegistry.ts
…rovenance Renames `src/license/vendor/` to `src/license/handsontable-license-key-parser/` so the directory names the repository it mirrors, and renames the paired test directory to match. Drops two provenance paragraphs that only described the older, superseded port rather than the code beside them. Corrects the divergence list, which is the part a drift review reads. Item 3 described upstream's own date check as if it were this port's, which the code has not matched since a type guard was added: upstream matches the stringified value against the date shape, so a value that merely spells a date once stringified is carried into a restricted entitlement, while here it takes the invalid-key path. That is the only divergence which changes what a key is worth, so it is now listed in its own right, with what the key specification says about the field and the note that upstream has not adopted it. The public guide no longer describes the key formats, and the validation section now points at the terms of the contract instead of enumerating the dates it compares. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 27a672c. Configure here.
The guide no longer documents what a proprietary key looks like, and the changelog entry carried the same description in the same words. It says what changed and what is unaffected instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@sequba One more thing in the vendored parser, found while acting on your comments rather than from your review — flagging it because it is the only divergence from upstream that changes which keys are accepted.
Two problems with how that was documented, both now fixed in
On the substance: the key spec's addendum (T7) makes that field a real calendar date, so the stricter reading is the specified one and it is upstream that deviates. Upstream has not adopted it — checked at |
A key can run out along two axes and only one of them is about the build in use. A maintenance key stops covering releases, so an older version keeps working and installing one is the fix; a usage-based key stops being valid at all, and telling its holder the key "is not valid for the installed version" sends them to downgrade, which changes nothing. Both went through one message, because the wording predates the usage axis this work introduced. The axis now reaches the message, and the classic 25-character format keeps the old wording, being release-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## hf-307-entitlement-gating-pr2 #1730 +/- ##
=================================================================
+ Coverage 97.34% 97.40% +0.05%
=================================================================
Files 198 204 +6
Lines 15850 16272 +422
Branches 3477 3602 +125
=================================================================
+ Hits 15429 15849 +420
- Misses 413 415 +2
Partials 8 8
🚀 New features to boost your workflow:
|
This branch has not been deployed
4 participants
Update 2026-09-18 — folded #1731, #1736, #1737, #1740, #1741 into this PR
So that no PR in this stack shows code a later PR rewrites, this PR now contains, squashed with
no loss of content: the entitlement reader (rev 6, was #1740), the notice-window warning (was
#1736), the two add-on tokens (was #1737), the function-metadata filtering (was #1731), and both
capability-token dialects (was #1741). Those five PRs are closed, folded here. The stack is now
#1728 → #1729 → this PR → #1743.Two things below are stale as a result and are corrected here rather than edited in place, so the
history stays legible: the title above (fixed), and every PR-number reference in the body below
still says
#1740/#1741/etc. — read those as "now #1730". The CHANGELOG entries in the diff arealready repointed to #1730.
Everything below this line is the PR as it was written across 2026-08-11 through 2026-08-26,
before the fold — the decisions and verification it documents did not change.
Third of four. Stacked on PR 2 (#1729), so review the commits on top of it.
Paired tests: handsontable/hyperformula-tests#32 — merge that first.
Why this PR exists at all
A genuine typed license key did not work before this.
checkLicenseKeyValidityrecognizes three fixed strings and the older 25-character format; a typed key matches none of them and falls through toINVALID. I verified this by building an engine with a real, unexpired[SUB]key before touching anything:So a paying customer pasting a valid key got
#LIC!in every cell plus a console warning. An entitlement adapter alone would not have helped — gate A kills the formulas first.What lands here
1. Vendor the typed-key reader into
src/license/handsontable-license-key-parser/(src/license/vendor/allowJsis off,strictis on, so this is a port, not a copy). This follows the key spec's own recommendation of a vendored copy plus a drift check rather than a shared package: a private dependency would breaknpm installfor open-source users of this GPL package.PROVENANCE.mdrecords the upstream commit and a per-file sha256, so drift is detectable by re-cloning and re-hashing, and lists the deliberate divergences.2. Resolve typed keys into both gates.
resolveLicensereads the key once and answers both gates from that single reading, so they cannot disagree about what the string says. Anything that is not a typed key —gpl-v3, an older-format key, an empty string, a typed key with a broken checksum — falls through tocheckLicenseKeyValiditywith its verdict unchanged. That function is extracted from rather than rewritten, so both paths share one warning-per-page flag instead of each getting their own. Two additive edits to the file it lives in, both affecting the legacy path too and both deliberate:formatDatenow reads its date in UTC (it printed the expiry a day early west of UTC — a latent bug on the legacy path as well), and an@internalreset for the warn-once flag was added because it made the console path untestable.3. The real capability table, plus reading both payload shapes.
Please confirm two things
The gate-A/gate-B invariant. Only a VALID typed key resolves to a restricted entitlement; missing, invalid and expired all resolve to
unrestrictedEntitlement(). Gate A already stops formula evaluation on its own — letting a bad key restrict the entitlement as well would make PR 2'sensureCapabilitythrow from the CRUD API, turning today's "formulas fail, the API still works" into a silent breaking change for every user whose key lapsed. Mutation-tested.The capability table is a DRAFT. Membership is transcribed from the packaging design's own per-function evidence file rather than invented, and the transcript is checked two ways: it reproduces that file's five published counts exactly (370 rows; 17 / 51 / 127 / 355 cumulative plus 15 operators), and all 423 registered function ids resolve into the 370 canonical entries once the 53 declared aliases are canonicalised, with zero uncovered. But the design is still under review — the free tier's exact contents and the placement of several function families are not settled. Landing it now was a deliberate call, not a claim that it is final.
Judgement calls worth a reviewer's eye
capabilities: the shipped shape (tier/addons/exp) and the newer specified shape (capabilities/usage_until/release_until/notice/flags). They disagree about nearly every field, the newer one is still under review, and only the first can be minted today. The newer spec also contradicts itself on whether its dates areYYYY-MM-DDstrings or numeric timestamps, so both are accepted.coretoken, which made feature gating inert by construction. The five features now live on their ownfeat:*tokens: a rev-5 key grants exactly thefeat:*tokens it carries; the shipped shape — whose vocabulary predates feature tokens — is granted all five by the adapter, so a shipped-shape key's API behaviour is unchanged; legacy keys resolve unrestricted (Kuba's carve-out). The exits stay ungated (resumeEvaluation, teardown), per the never-gate-the-exit rule.flags; the earlier coupling (silent || unrecognized > 0) suppressed strictly more than D3 asks for and was confirmed an implementation error by Kuba.spreadsheethas no agreed content — the packaging design names a package "Spreadsheet" and the pricing work names a "Spreadsheet Bundle" add-on, and whether those are the same set is unsettled; guessing either way silently sells an empty add-on or duplicates a whole tier. Pinned by a test so filling it has to be deliberate.noticeDaysis 0 for the shipped shape, which has no notice field. Inventing a default in the parser would put a product decision in the wrong place. The newer shape carriesnoticeand it is used.PROVENANCE.mdnames the upstream private repo and commit. That is what makes the documented drift check runnable. Ratified by Kuba on the task (D7-A, 12.08): kept as-is.Update 13.08 — key spec rev 5 read against the code
Reading rev 5 (doc, updated 12.08) and running its own example payload through this branch turned up three things, all fixed in the last commit:
functions_1..4,spreadsheet,import_export— there is nofeat:*entry at all. So a key that names no feature token cannot be saying "no features"; nothing in circulation can express one. Minting the spec's own §2 payload and running it:setCellContents,addRows,copy,undo,addNamedExpressionandbatchall threw. The same hole hit Handsontable-only keys and keys withhyperformula: null, which lost the API thatcoreused to give them, silently (gate-A VALID = no console warning). A key that does name afeat:*token still gets exactly what it names, which is the gating Kuba asked for.no-console-warnsis honoured. rev 5 spells the flag three ways — §2.3 and the §2 example sayno-console-warns, §4.3/§5.2 saysilent-console, earlier revisions saidsilent. Only the last two were recognised, so a doc-conformant SaaS key printed console warnings it had explicitly asked to suppress.capabilitiesrejects the key. Present-but-not-an-array fell through to the shipped-shape branch, which was a free pass twice: every feature granted, and the rev-5 dates never read — a subscription expired in 2020 resolved as perpetual.Still divergent from rev 5, deliberately: §4.1 says a non-trial key never hard-blocks, even past grace ("Trial: block. Non-trial: error only (18.1)"). This branch keeps EXPIRED →
#LIC!, per Kuba's D5=A (hard blocking stays this release; the notice/soft-stop/hard-stop model is a follow-up).Verification
Full unit suite green — 512 suites, 6344 tests (after the 13.08 update). Each of the three fixes above is mutation-verified: removing the opt-in rule fails 9 tests, removing
no-console-warnsfails 1, removing thecapabilitiesguard fails 2, and cutting the console notification fails 1 (it used to fail none). Beyond that, the parts that matter were mutation-tested rather than trusted: a wrong SHA-512 round constant, a one-bit UTF-8 error, a broken invariant and a moved package boundary each fail the tests that are supposed to catch them.Note
High Risk
Changes commercial licensing, formula evaluation (
#LIC!), and API capability checks across the engine; mistakes affect every customer key and entitlement path.Overview
This PR wires proprietary license keys to real feature packages and add-ons: entitlement keys (rev 6) are parsed via a vendored reader, resolved once in
resolveLicense, and mapped through a static capability table (functions_1–functions_4,fun:*group/per-function tokens,feat:*API areas, andspreadsheet/import_exportadd-ons).Runtime behavior: For a valid restricted key, gated built-in functions evaluate to
#LIC!and gated API areas throwLicenseCapabilityMissingError;getAvailableFunctions()/getFunctionDetails()only list functions the key actually grants. Invalid, missing, or expired keys still use an unrestricted entitlement for gate B (formulas fail via validity, but the public API is not further locked down). Legacy 25-character keys andgpl-v3are unchanged.Also added: one-time console messaging (invalid/expired plus “valid until … (UTC)” inside the usage notice window), optional silencing via config rebuild paths, UTC-correct expiry dates in warnings, and updated license/error docs.
Reviewed by Cursor Bugbot for commit dd731a9. Bugbot is set up for automated code reviews on this repo. Configure here.
⛔ Update 18.08 — upstream retired this key format the same day, and one claim above is wrong
A scoped spec-to-ship review of this PR (5 dimensions, every finding adversarially verified) confirmed 5 findings. Four are test-hardening and are fixed in the paired PR — see hyperformula-tests#32's "Update 18.08". The fifth needs a decision, not a patch, so nothing here has been changed for it yet.
The vendored reader cannot read the format upstream now ships.
handsontable/license-keycommit01eae6530d4f(2026-08-18 08:44 UTC, DEV-2512, "Replace typed license keys with the entitlement key format (rev 6)"), released as 4.0.0, deletessrc/typed-key/outright — the exact five filesPROVENANCE.mdpins and hashes. Its own docs are explicit: "The typed key format was removed, not deprecated… this package cannot read a typed key." The replacement differs in three ways this reader gates on:[SUB]_/[FREE]_/[TRIAL]_/[PERP]_tag (extractKeyData.ts:147)v: 1(extractKeyData.ts:199)vfield at all — payload is exactly{"products":{…}}extractKeyData.ts:105)Measured, not inferred: an authentic 4.0.0-format key built per upstream's own
build-payload.jsgivesextractTypedKeyData(key)→null,resolveLicense(key).validityState→invalid, andC1→#LIC!"License key is invalid.". Passing only the[…]block behaves the same. The retired tagged format still resolves correctly on the same build — so this is a format gap, not a robustness one.Independently, Budzio published the byte-level spec for that format this morning as a new page under key-spec rev 6 ("Technical implementation", sections
T1–T8, created 09:45 UTC). It matches upstream and confirms all four rows above. Three of its rules this branch already satisfies: checksum verified before any decode or parse ("order is normative"), uppercase hex rejected rather than normalised, and__proto__-safe result construction.What is genuinely at stake, and what is not. The invariant holds throughout: a rev-6 key resolves INVALID and therefore to
unrestrictedEntitlement(), so the CRUD API keeps working and only formulas fail (verified). And myHOT PR #73 (merged tomaster12:37 UTC today, pinninglicense-key#4.0.0) states "the typed keys never reached production", with the legacy 25-character keys still generated in parallel and validating exactly as before — that path is untouched here. So the correction owed to this PR's own opening claim: no customer ever held a typed key, so "a paying customer pasting a valid key got#LIC!" describes a format that was never issued. The live exposure is narrower but real — per that PR's surfacing table, trial orders and the HubSpot trial webhook now surface the entitlement key, while commercial orders and transactional emails still show the legacy key only.Why this is not fixed in this PR. Re-porting is the obvious move and upstream made it cheaper than the original port (
sha512.jsandutils.jsare unchanged; the newextractEntitlementKeyDatais deliberately schema-free and built to be vendored, so only the extractor, schema and constants need re-porting). But it is not a patch to slip in: adopting the new shape rules verbatim also makescapabilitiesmandatory and invalidates every payload shape this branch currently reads, rev 6 is still status "for review", and the work is arguably HF-329's (unassigned, due 21.08, untouched since 04.08 — it now has concrete content). Raised on the task as D9; the merge freeze until the spec owner returns means escalating is the action, not a delay.Also worth a reviewer's eye, since it sharpens the still-unanswered D8: rev 6's own examples use
capabilities: ["functions_1", "spreadsheet"]/["core"]— i.e. exactly the token names this branch implements — while the packaging design published 12.08 states its vocabulary isfun:<family>.<A|B|C>and that "nothing else about packaging exists at the technical layer". The two documents disagree, and this branch conforms to the key spec rather than to the packaging doc. A key minted in the packaging vocabulary resolves VALID with the whole gated API and zero functions, silently (gate A says VALID, so nothing prints, andunrecognizedCapabilitiesis exposed nowhere). That outcome is now pinned by a test in the paired PR — pinned, not endorsed, so whichever way D8 lands has to change it.Changed in the 2026-09-22 review round
PROVENANCE.md: two paragraphs about the supersededsrc/typed-key/port removed; the divergence list corrected. Its item 3 described upstream's date check as if it were this port's, which has not been true since a type guard was added, and that guard — the only divergence that changes which keys are accepted — was missing from the list entirely.developcan see; it is not for a subscription key.impliesfield and its recursive expansion are gone (see HF-307: entitlement model, capability registry, gate B (PR 1/4) #1728), so no capability token refers to another.