Skip to content

Commit 07c2290

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-22128-draft-head-resolution
2 parents 6c5740f + 959c209 commit 07c2290

20 files changed

Lines changed: 2242 additions & 84 deletions
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/plugin-security": minor
3+
---
4+
5+
feat(plugin-security)!: the six built-in positions are declared position metadata of the plugin (ADR-0131 D2)
6+
7+
Clause-②: yes (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) no authorable key, export or stored shape changes; the refusal is of six reserved position names at one REST door, and no stored definition is rewritten -->
10+
11+
**BREAKING** on one write door, shipped as `minor` under the repo's launch-window convention for breaking changes.
12+
13+
- **What is declared.** The identity positions `platform_admin`, `org_owner`, `org_admin` and `org_member` (ADR-0068 D2) and the audience anchors `everyone` and `guest` (ADR-0090 D5/D9) are registered with the engine registry as `position` metadata owned by `com.objectstack.plugin-security`, the way the plugin's own permission sets are. Their names, labels and descriptions come from one list built from `@objectstack/spec`'s `BUILTIN_IDENTITY_NAMES` and `BUILTIN_IDENTITY_METADATA` and the anchor text, and the built-in seeder reads that same list.
14+
- **Rows and grants do not move.** `sys_position` is seeded exactly as before: the same names, the same organizations (one copy per organization under a walled posture, one organization-less pass under `single`), `managed_by: 'platform'`, `active: true`, `is_default: false`, and the same labels and descriptions. No principal's grants change.
15+
- **What a client can now read.** `GET /api/v1/meta/position` lists the six beside the stack's own positions, and `GET /api/v1/meta/position/:name` answers `200` with each one's definition, where it answered `404 RESOURCE_NOT_FOUND`. The security catalog read (`createSecurityCatalogReader` in `@objectstack/core`) lists them from the engine registry.
16+
- **What a client can no longer write.** `PUT /api/v1/meta/position/:name` naming one of the six is refused `403 NOT_OVERRIDABLE`: the name is provided by a code package, and `position` has no overlay. It was saved as an environment-wide definition before. Give an authored position a different name. `DELETE /api/v1/meta/position/:name` on one of the six still answers `200` and removes nothing.
17+
- **The declared-positions seeder** keeps reading the engine registry first and the metadata service only when the registry holds no position. The six are left out of that decision and out of what it seeds, so stack-declared positions keep seeding and the six keep the built-in seeder as their one writer.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
'@objectstack/metadata': patch
3+
'@objectstack/runtime': patch
4+
---
5+
6+
fix(runtime,metadata): publishing or reverting a read-only package is refused with `422 WRITABLE_PACKAGE_REQUIRED`, and package membership reads the `_packageId` stamp
7+
8+
Clause-②: no
9+
10+
ADR-0070 D2 makes a code or installed package read-only. `POST /api/v1/packages/:id/publish` and `POST /api/v1/packages/:id/revert` did not check this. These two answers change:
11+
12+
- **Publish of a code or installed package: 200 → 422.** Before, the publish door answered 200. For a package whose items carry an authored `packageId`, for example `com.example.showcase`'s two capabilities, it answered `success: true` and wrote `publishedDefinition`, `state` and `version` onto those read-only items. For every other code package it answered `success: false`, "No metadata items found". It now answers `422 WRITABLE_PACKAGE_REQUIRED` before anything is written, the same refusal that `PATCH /packages/:id/disable` and `DELETE /packages/:id` already give.
13+
- **Revert of a code or installed package: 404 or 409 → 422.** Before, the revert door answered `404 RESOURCE_NOT_FOUND` "No metadata items found" (for example `com.objectstack.setup` and the platform packages that ship objects) or `409 RESOURCE_CONFLICT` "Package '…' has never been published" (for example `com.example.showcase`). It now answers `422 WRITABLE_PACKAGE_REQUIRED`. The check runs after the protocol's stored-row answer, so a revert of a code package that has a stored row bound to it, such as an organization overlay draft, keeps its answer.
14+
15+
To customise what a code package provides, use an ADR-0005 organization overlay. Overlay drafts publish through `POST /api/v1/packages/:id/publish-drafts` and the per-item publish door, and this change leaves both alone.
16+
17+
**Unchanged:** a writable package's publish and revert, and an id that nothing carries (revert 404; publish 200 with `success: false`).
18+
19+
`MetadataManager.publishPackage` and `revertPackage` now find a package's members by `packageId`, `package` or the private `_packageId` stamp. The artifact loader writes that stamp through `applyProtection`, and the ObjectQL object bridge copies it onto every object it registers. Before, an item that carried only the stamp was not a member. `MetadataManager` has no notion of package kind; the refusal of read-only packages lives at the two doors above.

‎.github/workflows/validate-deps.yml‎

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ on:
1717
# that governs it ever running on the PR that adds it.
1818
- 'osv-scanner.toml'
1919
- 'scripts/check-osv-exemptions.mjs'
20+
# The pull_request verdict of the OSV step (#22082): an edit to it must
21+
# be exercised on the PR that makes it.
22+
- 'scripts/osv-base-relative.mjs'
2023
# Re-run when the workflow itself changes, so edits to these gates are
2124
# exercised on the PR that introduces them.
2225
- '.github/workflows/validate-deps.yml'
@@ -32,6 +35,11 @@ jobs:
3235
validate:
3336
name: Validate Package Dependencies
3437
runs-on: ubuntu-latest
38+
# Measured over the 100 most recent completed runs: p50 1.3 min, max
39+
# 1.9 min (#22082). 15 leaves room for a cold pnpm store and two OSV
40+
# scans on a pull request, and stops a hung step long before GitHub's
41+
# 360-minute default would.
42+
timeout-minutes: 15
3543
permissions:
3644
contents: read
3745
issues: write
@@ -138,12 +146,111 @@ jobs:
138146
node scripts/check-osv-exemptions.mjs --self-test
139147
node scripts/check-osv-exemptions.mjs
140148
149+
# Two verdicts, one scanner pin (#22082, ruling A).
150+
#
151+
# `schedule` and `workflow_dispatch` judge `main` ABSOLUTELY: every
152+
# advisory the lockfile matches fails the step. That daily red is the
153+
# signal that files the finding card, and this step is unchanged by the
154+
# pull_request verdict below.
155+
#
156+
# A pull request is judged BASE-RELATIVELY: it answers for the advisories
157+
# it introduces, not for the ones `main` already carries -- those are
158+
# `main`'s to fix, and a pull request that changes no dependency cannot
159+
# fix them. The steps below scan the pull request's lockfile and the merge
160+
# base's, each under its own `osv-scanner.toml`, and
161+
# scripts/osv-base-relative.mjs fails only on an advisory the merge base
162+
# does not match; the others become notices naming the advisory and its
163+
# open finding card. A scan that does not reach a verdict fails closed.
164+
#
165+
# The pin is an untagged upstream commit after the v2.5.0 tag whose
166+
# action.yml runs the ghcr.io/google/osv-scanner-action:v2.5.0 image (the
167+
# comment here used to read v2.3.8). Keep all three `uses:` lines on ONE
168+
# sha: the two verdicts must come from the same scanner.
141169
- name: Audit dependencies for known vulnerabilities (OSV-Scanner)
142-
uses: google/osv-scanner-action/osv-scanner-action@f4cfcc01edc9c8b756a9b873b7a623ca674da51e # v2.3.8
170+
if: github.event_name != 'pull_request'
171+
uses: google/osv-scanner-action/osv-scanner-action@f4cfcc01edc9c8b756a9b873b7a623ca674da51e # untagged, v2.5.0 image
143172
with:
144173
scan-args: |-
145174
--lockfile=pnpm-lock.yaml
146175
176+
# The checkout is the pull request's test merge (refs/pull/N/merge) at
177+
# depth 1. Its first parent is the base-branch commit GitHub merged the
178+
# pull request into -- the merge base of the scanned tree and the base
179+
# branch -- so ONE depth-1 fetch of that sha is all the history this needs
180+
# (never fetch-depth: 0). When neither pnpm-lock.yaml nor osv-scanner.toml
181+
# differs from it, the pull request cannot introduce an advisory and no
182+
# scan runs. The ledger is part of the condition: a pull request that
183+
# deletes an exemption changes the verdict without touching the lockfile.
184+
- name: Stage the OSV comparison against the merge base (pull request)
185+
id: osv-stage
186+
if: github.event_name == 'pull_request'
187+
run: |
188+
node scripts/osv-base-relative.mjs --self-test
189+
mapfile -t parents < <(git cat-file -p HEAD | sed -n 's/^parent //p')
190+
if [ "${#parents[@]}" -ne 2 ]; then
191+
echo "::error title=OSV base-relative verdict unavailable::HEAD is not the pull request's two-parent test merge (${#parents[@]} parent(s) found), so there is no merge base to judge against"
192+
exit 1
193+
fi
194+
base="${parents[0]}"
195+
git fetch --no-tags --depth=1 origin "$base"
196+
echo "base=$base" >> "$GITHUB_OUTPUT"
197+
if git diff --quiet "$base" HEAD -- pnpm-lock.yaml osv-scanner.toml; then
198+
echo "::notice title=OSV-Scanner verdict (base-relative)::lockfile unchanged — inherits main's verdict. pnpm-lock.yaml and osv-scanner.toml are identical to the merge base ${base:0:12}, so this pull request introduces no advisory; main's daily scheduled scan judges that lockfile."
199+
echo "compare=false" >> "$GITHUB_OUTPUT"
200+
exit 0
201+
fi
202+
for side in base head; do
203+
if [ "$side" = base ]; then rev="$base"; else rev=HEAD; fi
204+
mkdir -p ".osv-compare/$side"
205+
git show "$rev:pnpm-lock.yaml" > ".osv-compare/$side/pnpm-lock.yaml"
206+
git show "$rev:osv-scanner.toml" > ".osv-compare/$side/osv-scanner.toml"
207+
done
208+
echo "compare=true" >> "$GITHUB_OUTPUT"
209+
210+
# Both scans may exit non-zero (findings), so neither stops the job; the
211+
# judge below reads each result file against its step's outcome.
212+
# `--config` is explicit because the scanner looks for osv-scanner.toml
213+
# only in the scanned lockfile's own directory (measured on 2.5.0: a
214+
# ledger one directory up is not applied).
215+
- name: OSV-Scanner on the merge base's lockfile (pull request)
216+
id: osv-scan-base
217+
if: steps.osv-stage.outputs.compare == 'true'
218+
continue-on-error: true
219+
uses: google/osv-scanner-action/osv-scanner-action@f4cfcc01edc9c8b756a9b873b7a623ca674da51e # untagged, v2.5.0 image
220+
with:
221+
scan-args: |-
222+
--config=.osv-compare/base/osv-scanner.toml
223+
--format=json
224+
--output-file=.osv-compare/base/results.json
225+
--lockfile=.osv-compare/base/pnpm-lock.yaml
226+
227+
- name: OSV-Scanner on the pull request's lockfile (pull request)
228+
id: osv-scan-head
229+
if: steps.osv-stage.outputs.compare == 'true'
230+
continue-on-error: true
231+
uses: google/osv-scanner-action/osv-scanner-action@f4cfcc01edc9c8b756a9b873b7a623ca674da51e # untagged, v2.5.0 image
232+
with:
233+
scan-args: |-
234+
--config=.osv-compare/head/osv-scanner.toml
235+
--format=json
236+
--output-file=.osv-compare/head/results.json
237+
--lockfile=.osv-compare/head/pnpm-lock.yaml
238+
239+
- name: Judge the pull request's OSV findings against the merge base's (pull request)
240+
if: steps.osv-stage.outputs.compare == 'true'
241+
env:
242+
BASE_OUTCOME: ${{ steps.osv-scan-base.outcome }}
243+
HEAD_OUTCOME: ${{ steps.osv-scan-head.outcome }}
244+
BASE_SHA: ${{ steps.osv-stage.outputs.base }}
245+
# Read-only use: the open-issue search that names an inherited
246+
# advisory's finding card. The verdict never depends on it.
247+
GITHUB_TOKEN: ${{ github.token }}
248+
run: |
249+
node scripts/osv-base-relative.mjs \
250+
--base-results .osv-compare/base/results.json --base-outcome "$BASE_OUTCOME" \
251+
--head-results .osv-compare/head/results.json --head-outcome "$HEAD_OUTCOME" \
252+
--base-sha "$BASE_SHA"
253+
147254
- name: List outdated packages
148255
if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
149256
run: |

‎packages/metadata/src/metadata-manager.ts‎

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1795,6 +1795,47 @@ export class MetadataManager implements IMetadataService {
17951795
}
17961796
}
17971797

1798+
/**
1799+
* [#22113] The registry items that belong to `packageId`, for the two
1800+
* package-wide writes ({@link publishPackage}, {@link revertPackage}).
1801+
*
1802+
* An item belongs to a package when any of three keys names it:
1803+
*
1804+
* - `packageId` — the publish envelope's own key, and what a caller that
1805+
* registers an item for a package writes;
1806+
* - `package` — the legacy package stamp;
1807+
* - `_packageId` — the private provenance stamp. `applyProtection`
1808+
* (`@objectstack/spec/shared`, ADR-0010 §3.7) writes it on every item a
1809+
* code-shipped artifact registers here, and the ObjectQL object bridge
1810+
* copies it onto every object it registers here. A code-shipped item
1811+
* carries ONLY this key, and it is the key the metadata protocol scopes
1812+
* this registry's items by when it serves a package's reads.
1813+
*
1814+
* Reading the first two alone made a package's publish and revert miss
1815+
* every item that carries only the stamp ("No metadata items found" while
1816+
* every read served them).
1817+
*
1818+
* ⛔ Not a writability check. This class has no package-kind concept, so it
1819+
* cannot tell a read-only code package (ADR-0070 D2) from a writable one,
1820+
* and with the stamp read it would publish a code package's items like any
1821+
* other. The refusal belongs to the caller that holds the predicate: the
1822+
* `POST /packages/:id/publish` and `/revert` doors (`@objectstack/runtime`)
1823+
* refuse a non-writable package with `422 WRITABLE_PACKAGE_REQUIRED` before
1824+
* calling either method.
1825+
*/
1826+
private collectPackageMembers(packageId: string): Array<{ type: string; name: string; data: any }> {
1827+
const members: Array<{ type: string; name: string; data: any }> = [];
1828+
for (const [type, typeStore] of this.registry) {
1829+
for (const [name, data] of typeStore) {
1830+
const meta = data as any;
1831+
if (meta?.packageId === packageId || meta?.package === packageId || meta?._packageId === packageId) {
1832+
members.push({ type, name, data: meta });
1833+
}
1834+
}
1835+
}
1836+
return members;
1837+
}
1838+
17981839
/**
17991840
* Publish an entire package:
18001841
* 1. Validate all draft items
@@ -1832,16 +1873,10 @@ export class MetadataManager implements IMetadataService {
18321873
const shouldValidate = options?.validate !== false;
18331874
const publishedBy = options?.publishedBy;
18341875

1835-
// Collect all items belonging to this package
1836-
const packageItems: Array<{ type: string; name: string; data: any }> = [];
1837-
for (const [type, typeStore] of this.registry) {
1838-
for (const [name, data] of typeStore) {
1839-
const meta = data as any;
1840-
if (meta?.packageId === packageId || meta?.package === packageId) {
1841-
packageItems.push({ type, name, data: meta });
1842-
}
1843-
}
1844-
}
1876+
// Collect all items belonging to this package — the same membership
1877+
// `revertPackage` reads. A read-only code package never gets here through
1878+
// the publish door, which refuses it first (see `collectPackageMembers`).
1879+
const packageItems = this.collectPackageMembers(packageId);
18451880

18461881
if (packageItems.length === 0) {
18471882
return {
@@ -2060,15 +2095,7 @@ export class MetadataManager implements IMetadataService {
20602095
* Restores all metadata definitions from their published snapshots.
20612096
*/
20622097
async revertPackage(packageId: string): Promise<void> {
2063-
const packageItems: Array<{ type: string; name: string; data: any }> = [];
2064-
for (const [type, typeStore] of this.registry) {
2065-
for (const [name, data] of typeStore) {
2066-
const meta = data as any;
2067-
if (meta?.packageId === packageId || meta?.package === packageId) {
2068-
packageItems.push({ type, name, data: meta });
2069-
}
2070-
}
2071-
}
2098+
const packageItems = this.collectPackageMembers(packageId);
20722099

20732100
// [#7559] ADR-0112 — both refusals below carry a DECLARED `code` + `status`.
20742101
// They are the ordinary answers to an ordinary request (revert a package id

‎packages/metadata/src/metadata-service.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,71 @@ describe('MetadataManager — IMetadataService Contract', () => {
822822
message: expect.stringContaining('has never been published'),
823823
});
824824
});
825+
826+
// [#22113] An item can carry ONLY the private `_packageId` stamp —
827+
// `applyProtection` writes it on every item an artifact registers, and the
828+
// ObjectQL object bridge copies it onto every object. Collecting members by
829+
// `packageId` / `package` alone answered 404 "No metadata items found" for a
830+
// package every read serves. (Whether the package may be reverted at all is
831+
// the door's question — it refuses a read-only one with 422 first.)
832+
it('finds a package by the _packageId stamp: never published ⇒ 409, not 404', async () => {
833+
await manager.register('object', 'code_item', {
834+
name: 'code_item', label: 'Code Item', _packageId: 'com.acme.code', _provenance: 'package',
835+
});
836+
837+
await expect(manager.revertPackage('com.acme.code')).rejects.toMatchObject({
838+
code: 'RESOURCE_CONFLICT',
839+
status: 409,
840+
message: "Package 'com.acme.code' has never been published",
841+
});
842+
});
843+
844+
it('membership is any of packageId, package or _packageId: each member with a snapshot is restored', async () => {
845+
const snapshot = (label: string) => ({ label });
846+
await manager.register('object', 'by_package_id', {
847+
name: 'by_package_id', packageId: 'com.acme.keys', publishedDefinition: snapshot('A'), state: 'draft',
848+
});
849+
await manager.register('object', 'by_package', {
850+
name: 'by_package', package: 'com.acme.keys', publishedDefinition: snapshot('B'), state: 'draft',
851+
});
852+
await manager.register('object', 'by_stamp', {
853+
name: 'by_stamp', _packageId: 'com.acme.keys', publishedDefinition: snapshot('C'), state: 'draft',
854+
});
855+
// Control: a different package's item is not a member.
856+
await manager.register('object', 'other_pkg', {
857+
name: 'other_pkg', _packageId: 'com.acme.other', publishedDefinition: snapshot('D'), state: 'draft',
858+
});
859+
860+
await manager.revertPackage('com.acme.keys');
861+
862+
for (const [name, label] of [['by_package_id', 'A'], ['by_package', 'B'], ['by_stamp', 'C']] as const) {
863+
const item = await manager.get('object', name) as any;
864+
expect({ name, state: item.state, metadata: item.metadata }).toEqual({ name, state: 'active', metadata: { label } });
865+
}
866+
const other = await manager.get('object', 'other_pkg') as any;
867+
expect(other.state).toBe('draft');
868+
expect(other.metadata).toBeUndefined();
869+
});
870+
});
871+
872+
// [#22113] `publishPackage` reads the same membership as `revertPackage`:
873+
// an item carrying only the stamp is a member. (Flipped from the pin that
874+
// held it OUT while nothing refused a read-only code package; the
875+
// `POST /packages/:id/publish` door now refuses one with
876+
// `422 WRITABLE_PACKAGE_REQUIRED` before this method runs.)
877+
describe('publishPackage — an item carrying only the _packageId stamp', () => {
878+
it('is a member: it is snapshotted and published', async () => {
879+
await manager.register('object', 'stamped_item', {
880+
name: 'stamped_item', label: 'Stamped Item', _packageId: 'com.acme.stamped', _provenance: 'package',
881+
});
882+
883+
const result = await manager.publishPackage('com.acme.stamped', { validate: false });
884+
885+
expect(result).toMatchObject({ success: true, itemsPublished: 1, version: 1 });
886+
const item = await manager.get('object', 'stamped_item') as any;
887+
expect(item.state).toBe('active');
888+
expect(item.publishedDefinition).toMatchObject({ name: 'stamped_item', label: 'Stamped Item' });
889+
});
825890
});
826891

827892
describe('getPublished', () => {

0 commit comments

Comments
 (0)