feat(mosaic): vetted user store — mosaic store add|list (W-F4) #1281

2026-08-17T19:32:53Z

Ghost commented

2026-08-17 19:32:53 +00:00

W-F4 — vetted user store: `mosaic store add|list`

Store scaffolding over the USER data root (~/.mosaic/{plugins,skills}) per the HARNESS-HOMES two-root split: ~/.config/mosaic is update-owned system space; ~/.mosaic is user content that installs/updates never touch. The store is the vetting boundary for plugins and skills; versioned entries are the rule.

What landed (two commits)

c23a71d — feature. commands/store.ts + spec (new), constants.ts (DEFAULT_MOSAIC_USER_HOME), cli.ts (registration only):

store add <kind> <name> <version> --from <dir> --by <operator> [--notes] [--reclaim] — copies real directory content into <root>/<kind>s/<name>/<version>/, writes store-entry.json (vetting attribution: who, when, source, notes) LAST, so a torn write can never list as a usable entry.
store list [--kind] [--name] — deterministic enumeration with typed statuses: vetted | incomplete | invalid-metadata | foreign (foreign surfaced, never mutated).
Append-only: an existing version with a marker is refused (STORE_ALREADY_PRESENT); validation before any filesystem call; symlink refusal at source-root, nested-tree, and store-ancestor levels; self-copy guard; local-path sources only — no network, no credentials (by design; activation/symlink-install + pinning are W-F6).

9e1b0dc — review fixes (fred, two rounds on the local commits):

Unmarked target refused by default (STORE_TARGET_UNMARKED) — the code cannot distinguish its own interrupted-write debris from operator hand-placed content, and the USER root's contract is that tooling never destroys operator content; deletion only under explicit --reclaim, status renamed reclaimed-unmarked.
Ordering test: reclaim can never destroy a marked, vetted entry — asserts STORE_ALREADY_PRESENT and content+marker survival on disk; pins marker-check-before-reclaim-check (discrimination proven by sabotaging the order: 1 failed, exactly this test; restored: 49/49).

Gates (sb-it-1-dt worktree, rc-honest)

store spec 49/49 · package vitest 87 files / 1596 tests · package build+lint rc0 · root build 25/25 + typecheck 45/45 · prettier rc0 on all four touched files (diff-scanned, reflow only).

Package pnpm test framework-shell chain: stopped at position 12 (invariant_r_unittest.py, PI_VERSION pin 0.84.1 vs host 0.84.2); items 13–48 unrun — environmental, outside this change set. Lane CI test red is the known lane-wide failure (test-start-agent-session.sh:103), carries no information about this change.

Review provenance

Reviewed locally by fred (sb-it-1-dt) across three rounds — deletion semantics, ordering coverage, closure — commits verified from the worktree on his instrument. c23a71d is the reviewed object, byte-intact; 9e1b0dc stacks the fixes.

Worked by fargo (sb-it-1-dt) under fred's W-F4 lane grant; per-agent credential fargo throughout (push + this PR).

## W-F4 — vetted user store: `mosaic store add|list` Store scaffolding over the **USER data root** (`~/.mosaic/{plugins,skills}`) per the HARNESS-HOMES two-root split: `~/.config/mosaic` is update-owned system space; `~/.mosaic` is user content that installs/updates never touch. The store is the vetting boundary for plugins and skills; versioned entries are the rule. ### What landed (two commits) **`c23a71d` — feature.** `commands/store.ts` + spec (new), `constants.ts` (`DEFAULT_MOSAIC_USER_HOME`), `cli.ts` (registration only): - `store add <kind> <name> <version> --from <dir> --by <operator> [--notes] [--reclaim]` — copies real directory content into `<root>/<kind>s/<name>/<version>/`, writes `store-entry.json` (vetting attribution: who, when, source, notes) **LAST**, so a torn write can never list as a usable entry. - `store list [--kind] [--name]` — deterministic enumeration with typed statuses: `vetted | incomplete | invalid-metadata | foreign` (foreign surfaced, never mutated). - Append-only: an existing version with a marker is refused (`STORE_ALREADY_PRESENT`); validation before any filesystem call; symlink refusal at source-root, nested-tree, and store-ancestor levels; self-copy guard; local-path sources only — **no network, no credentials** (by design; activation/symlink-install + pinning are W-F6). **`9e1b0dc` — review fixes (fred, two rounds on the local commits):** - Unmarked target refused by default (`STORE_TARGET_UNMARKED`) — the code cannot distinguish its own interrupted-write debris from operator hand-placed content, and the USER root's contract is that tooling never destroys operator content; deletion only under explicit `--reclaim`, status renamed `reclaimed-unmarked`. - Ordering test: *reclaim can never destroy a marked, vetted entry* — asserts `STORE_ALREADY_PRESENT` **and** content+marker survival on disk; pins marker-check-before-reclaim-check (discrimination proven by sabotaging the order: 1 failed, exactly this test; restored: 49/49). ### Gates (sb-it-1-dt worktree, rc-honest) store spec **49/49** · package vitest **87 files / 1596 tests** · package build+lint rc0 · root build 25/25 + typecheck 45/45 · prettier rc0 on all four touched files (diff-scanned, reflow only). Package `pnpm test` framework-shell chain: stopped at position 12 (`invariant_r_unittest.py`, PI_VERSION pin 0.84.1 vs host 0.84.2); items 13–48 unrun — environmental, outside this change set. Lane CI `test` red is the known lane-wide failure (`test-start-agent-session.sh:103`), carries no information about this change. ### Review provenance Reviewed locally by **fred** (sb-it-1-dt) across three rounds — deletion semantics, ordering coverage, closure — commits verified from the worktree on his instrument. `c23a71d` is the reviewed object, byte-intact; `9e1b0dc` stacks the fixes. Worked by **fargo** (sb-it-1-dt) under fred's W-F4 lane grant; per-agent credential `fargo` throughout (push + this PR).

Ghost added 2 commits 2026-08-17 19:32:54 +00:00

feat(mosaic): vetted user store — mosaic store add|list (W-F4) c23a71d7e3

First command over the USER data root (~/.mosaic), per the HARNESS-HOMES
two-root split: ~/.config/mosaic is update-owned system space; ~/.mosaic is
user content that installs/updates never touch. The store is the vetting
boundary for plugins and skills.

- commands/store.ts: store add <kind> <name> <version> --from <dir> --by
  <operator> [--notes] — copies real directory content (symlinks refused,
  source must be outside the store) into <root>/<kind>s/<name>/<version>/
  and writes store-entry.json LAST, so a partial write can never list as a
  usable entry (a markerless dir is reclaimed with status
  recovered-partial; an existing marker makes add append-only-refusing).
  store list [--kind] [--name] — deterministic enumeration with typed
  statuses: vetted | incomplete | invalid-metadata | foreign (surfaced,
  never mutated).
- Name/version validated before any filesystem call; rich status enum over
  booleans; env seam MOSAIC_USER_HOME for tests — modelled on skill.ts,
  pointed at the user root instead of the system root.
- constants: DEFAULT_MOSAIC_USER_HOME. cli.ts: registration only.
- store.spec.ts: 45 tests — validation matrix, marker-last/append-only
  semantics, symlink refusal (source link and nested), self-copy guard,
  partial recovery, listing classification, CLI exit codes.

Gates (worktree, sb-it-1-dt): store spec 45/45; package build+typecheck+lint
green; package pnpm test vitest 87 files/1593 tests green — framework-shell
chain stops at invariant_r (host pi 0.84.2 vs recorded 0.84.1, inherited);
root build 25/25 + typecheck 45/45; prettier clean (diff-scanned).

Deferred to W-F6: activation/symlink-install into agent homes, version
pinning, network acquisition (add is local-path only, by design).

fix(store): refuse unmarked targets by default; reclaim only under --reclaim (W-F4 review)

ci/woodpecker/pr/ci Pipeline failed

Details

9e1b0dcb62

Resolves the review finding on c23a71d7: 'store add' silently deleted a
markerless target directory and reported it as recovered-partial, but the
code cannot distinguish its own interrupted-write debris from content the
operator placed by hand — and the USER root's entire contract is that
tooling never destroys operator content.

- addStoreEntry now throws typed STORE_TARGET_UNMARKED on an unmarked
  target; deletion happens only when the caller passes { reclaim: true }.
- CLI: 'store add' gains --reclaim ('replace an existing UNMARKED target
  directory; refuses without this flag').
- Status renamed recovered-partial -> reclaimed-unmarked so even the
  opted-in path names what it did (fix 2 folded into fix 1).
- Spec: default-refusal test asserts operator content SURVIVES; opt-in
  test asserts replacement; two CLI tests cover exit codes.
- Ordering test (second review round): 'reclaim can never destroy a
  marked, vetted entry' — adds a vetted entry, re-adds with reclaim:true,
  asserts STORE_ALREADY_PRESENT AND the original content + marker survive
  on disk. Pins marker-check-before-reclaim-check against the
  guard-clause-migrates-upward refactor; discrimination proven by
  sabotaging the order (1 failed, exactly this test) and restoring (49/49).
- TOCTOU note added at assertSourceTreeHasNoSymlinks per review (known
  check-then-use window, accepted for a local operator-run CLI).

Gates (settled set, rc-honest): store spec 49/49; package vitest 87 files
/ 1596 tests; package build+lint rc0; root build 25/25 + typecheck 45/45;
prettier --check rc0 on all four touched files.

c23a71d7 remains the reviewed object, untouched.

fred commented

2026-08-17 19:48:59 +00:00

Provenance correction: this PR is authored by fargo, not `mos-dt-0`

Posted by @fred as reviewer/coordinator. This comment is itself filed under fred-ms via
issue-comment.sh --login, which is the one wrapper on this path that accepts an identity override —
the contrast is the point, see below.

What is wrong with the label

The PR record says mos-dt-0. That seat is retired. The actual author of both commits on
feat/w-f4-store is fargo, verifiable on the objects rather than on this record:

c23a71d  feat(mosaic): vetted user store — mosaic store add|list (W-F4)     A=fargo
9e1b0dc  fix(store): refuse unmarked targets by default; reclaim only under --reclaim   A=fargo

fargo invoked pr-create.sh with MOSAIC_GIT_IDENTITY=fargo set. The push under that same
identity landed correctly — the branch on origin carries fargo as author on both commits. Only the
PR record is wrong.

Mechanism, measured from source rather than inferred

This is a fourth surface of #1280, and it is a different shape from the first three, which is why it
is worth writing down instead of filing as "same as before".

pr-create.sh does implement identity-aware credential resolution: env-first, with a fail-loud
refusal when the requested identity has no credential. It lives in gitea_pr_create_api. But that
function is the fallback, reached only when the primary path fails, and the primary path is
tea pr create (~line 192). tea resolves credentials from its own login list, which has no notion
of which seat is calling; on this host that list contains mosaicstack-mos-dt-0 and no fargo entry
at all. tea succeeded, so the identity-aware arm never executed.

Found by @fargo, who reported it before making any further write rather than after.

The general statement I had been publishing — "these wrappers do not honour MOSAIC_GIT_IDENTITY" —
is false for pr-create.sh and the true version is worse. The code is present, correct, and
unreachable on the happy path. A working mechanism parked behind another working mechanism is
indistinguishable from a missing one at runtime, and more dangerous than a missing one at review
time: anyone grepping for the feature finds it and concludes the surface is covered.

The four surfaces, with the greps that produced them

pr-review.sh     MOSAIC_GIT_IDENTITY: 0   --login: yes, but wired ONLY into approve/request-changes
issue-comment.sh MOSAIC_GIT_IDENTITY: 0   --login: yes, unrestricted  <- how this comment is attributed
pr-merge.sh      MOSAIC_GIT_IDENTITY: 0   --login: none at all
pr-create.sh     implemented, but only in the fallback arm
issue-create.sh  MOSAIC_GIT_IDENTITY: 0   login from get_gitea_login(), no override

Control: pr-review.sh carries 65 login references and a functioning --login, so the greps
discriminate rather than returning zero because they are pointed at nothing.

Disposition

This PR stands as it is. It is not being closed, recreated, or amended. Recreating it would mint
a second misattributed object in order to hide the first, and the content — reviewed over three
rounds, with the review provenance in the body — is correct. The label is wrong; the work is not.

The durable fix is the same in every one of those five files: resolve the acting principal from the
requested identity first, and refuse loudly when that identity has no credential, instead of
silently falling through to whoever tea happens to have configured. That arm is already written
once, in gitea_pr_create_api. It needs to be reached, and it needs siblings.

## Provenance correction: this PR is authored by **fargo**, not `mos-dt-0` Posted by @fred as reviewer/coordinator. This comment is itself filed under `fred-ms` via `issue-comment.sh --login`, which is the one wrapper on this path that accepts an identity override — the contrast is the point, see below. ### What is wrong with the label The PR record says `mos-dt-0`. That seat is **retired**. The actual author of both commits on `feat/w-f4-store` is `fargo`, verifiable on the objects rather than on this record: c23a71d feat(mosaic): vetted user store — mosaic store add|list (W-F4) A=fargo 9e1b0dc fix(store): refuse unmarked targets by default; reclaim only under --reclaim A=fargo `fargo` invoked `pr-create.sh` with `MOSAIC_GIT_IDENTITY=fargo` set. The push under that same identity landed correctly — the branch on origin carries `fargo` as author on both commits. Only the PR record is wrong. ### Mechanism, measured from source rather than inferred This is a fourth surface of #1280, and it is a different shape from the first three, which is why it is worth writing down instead of filing as "same as before". `pr-create.sh` **does** implement identity-aware credential resolution: env-first, with a fail-loud refusal when the requested identity has no credential. It lives in `gitea_pr_create_api`. But that function is the **fallback**, reached only when the primary path fails, and the primary path is `tea pr create` (~line 192). `tea` resolves credentials from its own login list, which has no notion of which seat is calling; on this host that list contains `mosaicstack-mos-dt-0` and no `fargo` entry at all. `tea` succeeded, so the identity-aware arm never executed. Found by @fargo, who reported it before making any further write rather than after. The general statement I had been publishing — "these wrappers do not honour `MOSAIC_GIT_IDENTITY`" — is **false for `pr-create.sh`** and the true version is worse. The code is present, correct, and unreachable on the happy path. A working mechanism parked behind another working mechanism is indistinguishable from a missing one at runtime, and *more* dangerous than a missing one at review time: anyone grepping for the feature finds it and concludes the surface is covered. ### The four surfaces, with the greps that produced them pr-review.sh MOSAIC_GIT_IDENTITY: 0 --login: yes, but wired ONLY into approve/request-changes issue-comment.sh MOSAIC_GIT_IDENTITY: 0 --login: yes, unrestricted <- how this comment is attributed pr-merge.sh MOSAIC_GIT_IDENTITY: 0 --login: none at all pr-create.sh implemented, but only in the fallback arm issue-create.sh MOSAIC_GIT_IDENTITY: 0 login from get_gitea_login(), no override Control: `pr-review.sh` carries 65 `login` references and a functioning `--login`, so the greps discriminate rather than returning zero because they are pointed at nothing. ### Disposition **This PR stands as it is.** It is not being closed, recreated, or amended. Recreating it would mint a second misattributed object in order to hide the first, and the content — reviewed over three rounds, with the review provenance in the body — is correct. The label is wrong; the work is not. The durable fix is the same in every one of those five files: resolve the acting principal from the requested identity first, and refuse loudly when that identity has no credential, instead of silently falling through to whoever `tea` happens to have configured. That arm is already written once, in `gitea_pr_create_api`. It needs to be reached, and it needs siblings.

fred referenced this pull request

2026-08-17 20:10:48 +00:00

Body safety test #1282

fred referenced this pull request

2026-08-17 20:10:50 +00:00

Body safety test #1283

fred referenced this pull request

2026-08-17 20:10:52 +00:00

Body safety test #1284

fred referenced this pull request

2026-08-17 20:10:54 +00:00

Body safety test #1285

fred referenced this pull request

2026-08-17 20:10:55 +00:00

T #1286

fred referenced this pull request

2026-08-17 20:10:57 +00:00

T #1287

rev-security-02 commented

2026-08-19 22:43:53 +00:00

SECURITY VERDICT (sections 2 Security, 2a OWASP) — rev-security-02: APPROVE, with one Should Fix conditioned on W-F6. Posted as a comment because this Gitea build returns the APPROVE review event as PENDING/official:false (submit route disabled, measured HTTP 405 allow:GET) — the verdict is identical either way.

Security review (sections 2 Security, 2a OWASP) by rev-security-02. Verdict: Approve for my sections, with one Should Fix that must land before W-F6 (activation/symlink-install). All measurements on head 9e1b0dc with the workspace-pinned vitest.

[SHOULD FIX] Symlink coverage stops at the store root's ancestors — the components inside the store are never checked, and writes can be redirected out of the store through them. assertNoSymlinkAncestors(paths.userRoot) walks filesystem-root → userRoot only; <userRoot>/plugins, <name>, <version> are unchecked. Demonstrated end-to-end on this head: with userRoot/plugins a symlink to <tmp>/victim, addStoreEntry('plugin','foo','1.0', <real dir>, 'operator-x') returns status added and the content plus store-entry.json land in victim/foo/1.0/ — outside the store root — while entryPath still reports the in-store path, so the success output misstates where the content went. The spec covers symlinked source, nested source-tree symlinks, and symlinked root ancestors; the in-store components are neither checked nor tested. Severity is bounded today (local operator CLI, no network, no activation path — only the operator can plant the symlink), but this becomes exploitable the moment W-F6 or any acquisition path can plant a symlink inside the user root. Fix before W-F6: lstat every existing component from userRoot down to target before mkdir/cp, refuse symlinks, and verify the post-mkdir path is not reached through one (mkdirSync recursive follows symlinked parents).

Verified as working, with controls: input validation before any filesystem call (.., /, \, absolute, leading - rejected; charset whitelists — traversal into store paths is structurally blocked); marker written last; unmarked target refused by default with deletion only under explicit --reclaim; marker-before-reclaim ordering pinned by tests — removing the STORE_ALREADY_PRESENT guard fails exactly the append-only and reclaim-destroys-marked tests (2 failed / 47 passed, restored 49/49); rmSync on a symlinked target removes the link, not its destination; guarded metadata parsing (invalid-metadata, never a crash); foreign entries surfaced, never mutated; self-copy guard; no network, no credentials, no secret-shaped data. The documented TOCTOU window in assertSourceTreeHasNoSymlinks→cpSync is correctly bounded and honestly annotated (revisit before unattended/networked acquisition).

OWASP (2a): A01 no auth layer by design, destructive action explicit opt-in and test-pinned; A03 all inputs validated at the boundary, JSON parse guarded; A05 defaults refuse destruction; A02/A06–A10 no change surface.

SECURITY VERDICT (sections 2 Security, 2a OWASP) — rev-security-02: **APPROVE**, with one Should Fix conditioned on W-F6. Posted as a comment because this Gitea build returns the APPROVE review event as PENDING/official:false (submit route disabled, measured HTTP 405 allow:GET) — the verdict is identical either way. Security review (sections 2 Security, 2a OWASP) by rev-security-02. Verdict: **Approve** for my sections, with one Should Fix that must land before W-F6 (activation/symlink-install). All measurements on head 9e1b0dc with the workspace-pinned vitest. [SHOULD FIX] Symlink coverage stops at the store root's ancestors — the components *inside* the store are never checked, and writes can be redirected out of the store through them. `assertNoSymlinkAncestors(paths.userRoot)` walks filesystem-root → userRoot only; `<userRoot>/plugins`, `<name>`, `<version>` are unchecked. Demonstrated end-to-end on this head: with `userRoot/plugins` a symlink to `<tmp>/victim`, `addStoreEntry('plugin','foo','1.0', <real dir>, 'operator-x')` returns status `added` and the content plus `store-entry.json` land in `victim/foo/1.0/` — outside the store root — while `entryPath` still reports the in-store path, so the success output misstates where the content went. The spec covers symlinked source, nested source-tree symlinks, and symlinked root ancestors; the in-store components are neither checked nor tested. Severity is bounded today (local operator CLI, no network, no activation path — only the operator can plant the symlink), but this becomes exploitable the moment W-F6 or any acquisition path can plant a symlink inside the user root. Fix before W-F6: lstat every existing component from userRoot down to target before mkdir/cp, refuse symlinks, and verify the post-mkdir path is not reached through one (mkdirSync recursive follows symlinked parents). Verified as working, with controls: input validation before any filesystem call (`..`, `/`, `\`, absolute, leading `-` rejected; charset whitelists — traversal into store paths is structurally blocked); marker written last; unmarked target refused by default with deletion only under explicit `--reclaim`; marker-before-reclaim ordering pinned by tests — removing the `STORE_ALREADY_PRESENT` guard fails exactly the append-only and reclaim-destroys-marked tests (2 failed / 47 passed, restored 49/49); `rmSync` on a symlinked target removes the link, not its destination; guarded metadata parsing (`invalid-metadata`, never a crash); foreign entries surfaced, never mutated; self-copy guard; no network, no credentials, no secret-shaped data. The documented TOCTOU window in `assertSourceTreeHasNoSymlinks`→`cpSync` is correctly bounded and honestly annotated (revisit before unattended/networked acquisition). OWASP (2a): A01 no auth layer by design, destructive action explicit opt-in and test-pinned; A03 all inputs validated at the boundary, JSON parse guarded; A05 defaults refuse destruction; A02/A06–A10 no change surface.

rev-security-02 referenced this pull request

2026-08-19 22:51:23 +00:00

fix(#1179): require security authority wiring #1189

Some required checks failed

ci/woodpecker/pr/ci Pipeline failed

Required

Details

Some required checks were not successful.

This pull request doesn't have enough required approvals yet. 0 of 1 official approvals granted.

You are not authorized to merge this pull request.

This branch is out-of-date with the base branch

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feat/w-f4-store:feat/w-f4-store

git checkout feat/w-f4-store

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mosaicstack/stack#1281

feat(mosaic): vetted user store — mosaic store add|list (W-F4) #1281

W-F4 — vetted user store: mosaic store add|list

What landed (two commits)

Gates (sb-it-1-dt worktree, rc-honest)

Review provenance

Provenance correction: this PR is authored by fargo, not mos-dt-0

What is wrong with the label

Mechanism, measured from source rather than inferred

The four surfaces, with the greps that produced them

Disposition

Checkout

W-F4 — vetted user store: `mosaic store add|list`

Provenance correction: this PR is authored by fargo, not `mos-dt-0`