Repository navigation
Commit 1ac7308
fix(core)!: the plugin artifact signature contract refuses any key that is not Ed25519 (#21534)
Fixes #21524
Clause-②: no (narrowing)
The plugin artifact signature contract in `@objectstack/core` declares
Ed25519 and now enforces it. Before this change, `signPayload` and
`verifyPayload` handed any key to node's `sign(null, …)` / `verify(null,
…)`, and those follow the key. So an RSA, EC or Ed448 key signed under
the `ed25519:KEYID:SIG` label and verified against its own public half.
Measured at `ad7c351898`, through the module and five key input shapes:
an RSA key produced a 342-character signature, EC 94 to 96, Ed448 152,
and Ed25519 86. Every one was labelled `ed25519` and every one verified
`true`.
## What changed
`packages/core/src/security/plugin-artifact-signature.ts`:
- **One check, `requireSignatureKeyType(key, alg, use)`.** A key passes
only when `key.asymmetricKeyType` IS the algorithm. Otherwise it throws
a plain `Error`, the module's existing style, naming the key type found
(`rsa`, `ec`, `ed448`, or `secret` for a symmetric key).
- **`signPayload`** calls the check with `SIGNATURE_ALG` before signing.
- **`verifyPayload`** calls it with `parsed.alg`, the algorithm the
signature's own label names. So the label is checked against the
verifying key's type instead of being trusted. `parseSignature` admits
only the `ed25519` label, so the same comparison is also the refusal of
every non-Ed25519 key. It is one rule, not two.
- **Key normalisation** is now "a `KeyObject` is used as given, anything
else goes through `createPrivateKey` / `createPublicKey`". It used to be
"a string is parsed, anything else is passed through". This keeps every
input node accepted working for an Ed25519 key, measured: PEM string,
`KeyObject`, PEM buffer, DER object and JWK object, at sign and at
verify. Without it, a PEM buffer would have reached the check with no
key type.
- **`verifyPublisherSignature`, `verifyPlatformSignature` and
`verifyPluginArtifact`** verify through `verifyPayload` and get the
check from there. Only their docblocks changed.
**Why `verifyPayload` throws where it used to answer `false`.** The card
asks for this to be justified. The ruling requires the refusal to be
loud and to name the key type, and a boolean can carry neither. The
trust paths below show that the verifying key always comes from the
caller's own configuration, never from the artifact. A non-Ed25519 key
is therefore a misconfigured trust anchor, not a verdict on the signed
bytes. Folded into `false`, it would read exactly like a tampered
artifact. Every case that answered `false` before still does: a
malformed signature string, a key that cannot be parsed, and a signature
that does not verify.
**`os plugin sign`: no edit to
`packages/cli/src/commands/plugin/sign.ts`.** Measured: its existing
`catch` around `signPayload` (`sign.ts:81-87` at `ad7c351898`) already
prints `✗ Signing failed: …` and calls `this.exit(1)` from the `catch`
body, outside the `try`. So the refusal surfaces as one error line and
exit 1 with no source change. PR #21522, which edits that file, has
since landed (`5895119c35`). Its change is in the self-verification
`catch`, not on this path. The CLI pin was also run against main's
landed `sign.ts`: 3/3 passed (see Tests).
**Changeset:** `@objectstack/core` minor, BREAKING, `Clause-②: no
(narrowing)`, ADR-0087 `not-required (no-migration-prescription)`.
`check:adr-0087-registration` reads it and passes.
## Trust paths of `verifyPluginArtifact`
Read at `ad7c351898`, in
`packages/core/src/security/plugin-artifact-signature.ts` unless named
otherwise.
1. **Platform key**: `keys.platformPublicKey` (`:212`), used at
`:228-229`, then `verifyPlatformSignature` (`:171`), then
`verifyPayload`. This is caller configuration.
2. **Publisher key**: `keys.getPublisherPublicKey(keyId)` (`:213`),
passed at `:219-222` to `verifyPublisherSignature` (`:148`), and
resolved at `:162`. The resolver is the caller's key registry. The
artifact contributes only the `keyId`, read out of its own signature
string by `parseSignature` (`:81`). That id selects an entry in the
caller's registry. An id the registry does not know is refused (`:163`),
and no resolver means `verified: false` (`:158-160`). The artifact
cannot introduce a key.
3. **Fields read from the artifact or its version record**: `signature`,
`platform_signature`, `package_id`, `version` and `blob_key`. These are
signature strings and identity only. No field carrying a key is read.
4. **`packages/core/src/plugin-loader.ts:512-536`**
(`verifyPluginSignature`) reads `plugin.signature` through
`parseSignature` for well-formedness and logs `alg` / `keyId`. There is
no key and no cryptographic check.
5. **Production callers of `verifyPluginArtifact` in this repository:
none.** `git grep` finds the module, the barrel re-export
(`security/index.ts:34`) and the module's test. This matches ADR-0025's
status line, which says there is no `.osplugin` loader and no runtime
path on which a distributed plugin executes.
6. **`os plugin sign`'s self-check** (`sign.ts:95-96`) verifies with the
public half derived from the private key it just signed with. That is a
self-consistency check, not a trust path.
**Against triage's raise rule:** no path admits a key supplied with the
artifact. The one input the artifact chooses is the `keyId`, a selector
over the caller's registry that cannot reach a key outside it. This is a
reading, stated for the seat to grade.
## Cloud counterpart
NOT MEASURED: no read access to objectstack-ai/cloud from this session.
The module header claims byte-for-byte compatibility with the cloud
control plane's signing module, and this PR did not read that module.
Whether it has the same gap, and the bare finding to file there if it
does, is left to a seat with that access. For an Ed25519 key this side's
output is unchanged: the signature bytes are deterministic and identical
before and after.
## Tests (at `f1f4368020`)
- `pnpm --filter @objectstack/core exec vitest run --project local
src/security/plugin-artifact-signature.test.ts`: **24 passed**, 14
existing and 10 new.
- An RSA key and an EC key are each refused at `signPayload`, as PEM and
as `KeyObject`.
- Each is refused at `verifyPayload` with a signature that verifies
cryptographically, so the check is the only thing refusing it.
- Each is refused on both `verifyPluginArtifact` trust paths: the
publisher registry and the platform key.
- An Ed25519 key signs and verifies as before. A PEM signature and a
`KeyObject` signature are byte-identical.
- A label that disagrees with the key type is refused both ways: an
`ed25519` label over an RSA key throws, and an `rsa` label answers
`false`.
- A symmetric key is refused as `secret`. An unreadable key and a
malformed signature still answer `false`.
- `pnpm --filter @objectstack/core test`: 76 files, **2166 passed**.
`pnpm --filter @objectstack/core typecheck`: exit 0. The test file is in
the `tsconfig.test.json` program (`--listFiles`: 1 hit).
- `pnpm --filter @objectstack/cli exec vitest run --project unit` over
`test/plugin-sign.test.ts`, `test/plugin-commands.test.ts`,
`test/plugin-publish.test.ts`, `test/plugin-publish-visibility.test.ts`
and `test/json-exit-signal.pin.test.ts`: 5 files, **100 passed**. The
new pin, for an RSA key and an EC key, asserts four things: exit 1,
exactly one error line, that line being `signPayload`'s refusal naming
the key type, and no `Plugin signed` and no sidecar.
- `pnpm --filter @objectstack/cli typecheck`: exit 0. The CLI
`integration` tier is left to CI: the diff touches no integration-tier
file and no spawn entry.
- **Joint state:** `sign.ts` was swapped to main's landed copy
(`550f4cc2fd`, which carries PR #21522) and then restored. The blob
matched HEAD and `git diff HEAD` was empty. The CLI pin passed 3/3.
- The CLI suite resolves `@objectstack/core` through `dist/`, so `core`
was rebuilt before every CLI reading.
## Ablation and reverse verification
Each leg ran on committed code, through `scripts/ablation-replace.mjs`.
Each restore was proven by blob hash equal to HEAD and an empty `git
diff HEAD`. Every leg below went red as predicted, except C's first run,
which is described under C.
- **A**: delete the `signPayload` check. The core suite resolves source.
**3 red**: RSA sign, EC sign and `secret`. 21 green.
- **B**: delete the `verifyPayload` check. **5 red**: RSA and EC verify,
RSA and EC trust paths, and the label case. 19 green.
- **C**: delete the `signPayload` check, measured at the CLI. Core was
rebuilt, and `ablation-dist-preflight` confirmed the marker absent from
`dist/`. **Observed direction, first run: the CLI pin stayed green.**
The verify-side check refused the same key at the self-verification
step, so the command still exited 1 with one error naming `rsa`. The two
checks overlap at this door. The pin was then tightened to assert that
the refusal is `signPayload`'s, given at sign time. Re-run: **2 red**
(`expected '✗ Self-verification error: verifyPayload…' to match
/signPayload: the private key is of type …/`).
- **D**: make the one check a no-op, measured at the CLI. The
`ablation-d-noop` marker was present in `dist/`. **2 red**: `expected
undefined to be 1`. The command completed with exit 0, which reproduces
the card's measurement at the public door.
- **Restore leg:** core rebuilt. Preflight found the D marker absent and
the original check present, and the tree clean against HEAD. The CLI pin
passed 3/3.
## Gates
- `node scripts/pm/dispatch-gates.mjs --commands`, run without paths on
the final diff: 64 commands, tree `f1f4368020`, merge base `ad7c35189`.
All 64 exited 0.
- `check:dual-build-cjs-loads` and `check:lean-entry-closure` first
answered exit 3 PREREQUISITE NOT MET. Both were re-run green after a
full turbo build (72 tasks, 71 cached).
- `--ran` reconciliation: 64 derived, 64 run, 0 NOT-MEASURED. That zero
is derived, because every line carries its exit code.
- The PM's lead list also named `check:i18n`, `check:i18n-coverage` and
`check:i18n-walk-parity`. The derivation on this diff does not, because
no `packages/cli/src` path changed. They were not run.
**Lint, a declared narrowing:** `pnpm exec eslint --no-inline-config
--format json` on the three touched TypeScript files gave 3 files, 0
errors and 0 warnings.
- **Population:** the config's `files:
['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']`. The changeset is outside it.
- **Invariance:** `eslint.config.mjs` enables no type-aware linting. It
sets no `parserOptions.project` and no typed rules, and every
`parserOptions` block is `ecmaVersion` and `sourceType` only. Its only
disk reads are two baseline JSON files this diff does not touch. So the
diff cannot move the verdict on an untouched file.
## Acceptance notes
- **`PluginSignatureVerifier`**
(`packages/core/src/security/plugin-signature-verifier.ts`) is a second
plugin-signature verifier, exported from the same barrel. It declares
`algorithm: 'RS256' | 'ES256'` and verifies with
`createVerify('RSA-SHA256')` / `createVerify('sha256')`, where the key,
not the declared algorithm, again decides the scheme. Measured with node
directly, not through the class: `createVerify('RSA-SHA256')` verifies
an ECDSA signature against an EC key as `true`. It also expects a bare
base64 signature, while `plugin-loader.ts` requires
`PluginMetadata.signature` to parse as `ed25519:KEYID:SIG`. It has zero
production callers: the barrel and one pin test that asserts it is
exported. The class was read, not run, and has no live reach, so it is
noted and not filed. No carrier.
- `verifyPayload` still answers `false` for a key it cannot parse at
all, which is the trust configuration being wrong in a different way. It
is left as it was, under the card's rule of no throw where `false` was
answered before, and noted only. No carrier.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DDZNkDVwPQnevTFcYE47H3)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 4916168 commit 1ac7308
4 files changed
Lines changed: 242 additions & 9 deletions
File tree
- .changeset
- packages
- cli/test
- core/src/security
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
4 | 6 | | |
5 | 7 | | |
6 | 8 | | |
| |||
59 | 61 | | |
60 | 62 | | |
61 | 63 | | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
4 | | - | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
5 | 13 | | |
6 | 14 | | |
7 | 15 | | |
| |||
145 | 153 | | |
146 | 154 | | |
147 | 155 | | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
23 | 30 | | |
24 | 31 | | |
25 | 32 | | |
| |||
30 | 37 | | |
31 | 38 | | |
32 | 39 | | |
33 | | - | |
| 40 | + | |
34 | 41 | | |
35 | 42 | | |
36 | 43 | | |
37 | 44 | | |
38 | 45 | | |
39 | 46 | | |
40 | 47 | | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
41 | 51 | | |
42 | | - | |
| 52 | + | |
43 | 53 | | |
44 | 54 | | |
45 | | - | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
46 | 79 | | |
47 | 80 | | |
48 | 81 | | |
| |||
60 | 93 | | |
61 | 94 | | |
62 | 95 | | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
63 | 99 | | |
64 | 100 | | |
65 | 101 | | |
66 | 102 | | |
67 | 103 | | |
68 | 104 | | |
69 | 105 | | |
70 | | - | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
71 | 109 | | |
72 | 110 | | |
73 | 111 | | |
| |||
93 | 131 | | |
94 | 132 | | |
95 | 133 | | |
96 | | - | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
97 | 144 | | |
98 | 145 | | |
99 | 146 | | |
100 | 147 | | |
101 | 148 | | |
102 | 149 | | |
103 | 150 | | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
104 | 158 | | |
105 | | - | |
| 159 | + | |
106 | 160 | | |
107 | 161 | | |
108 | 162 | | |
| |||
144 | 198 | | |
145 | 199 | | |
146 | 200 | | |
| 201 | + | |
| 202 | + | |
147 | 203 | | |
148 | 204 | | |
149 | 205 | | |
| |||
167 | 223 | | |
168 | 224 | | |
169 | 225 | | |
170 | | - | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
171 | 230 | | |
172 | 231 | | |
173 | 232 | | |
| |||
196 | 255 | | |
197 | 256 | | |
198 | 257 | | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
199 | 263 | | |
200 | 264 | | |
201 | 265 | | |
| |||
0 commit comments