98 lines
4.5 KiB
Markdown
98 lines
4.5 KiB
Markdown
# Issue #1264 Code and Security Review
|
|
|
|
> Branch: `fix/1264-fleet-unattended-first-start` | Base:
|
|
> `origin/next@476db12b92971634b67fd2057b7577ee5894e449`
|
|
|
|
## Initial automated review
|
|
|
|
Codex reviewed the pre-PR uncommitted delta with:
|
|
|
|
```bash
|
|
~/.config/mosaic/tools/codex/codex-code-review.sh --uncommitted \
|
|
-o /tmp/1264-codex-code-review.json
|
|
```
|
|
|
|
Result: `request-changes`, confidence `0.93`, 20 files, one should-fix. `checkSoul()` trimmed
|
|
`MOSAIC_AGENT_NAME` for pre-seed resolution while composition used the original value, so a padded
|
|
name could seed files before later refusal.
|
|
|
|
Remediation rejected blank/leading/trailing-whitespace values before roster lookup or writes and
|
|
added three built-CLI no-side-effect regressions. Automated re-review approved that delta with no
|
|
findings (confidence `0.86`). Initial security review reported risk `none` (confidence `0.91`).
|
|
|
|
## Formal exact-head review
|
|
|
|
Daphne reviewed PR #1268 at exact head `43fa0477877e0d0f110da8d11c3033b40ddeb191` and filed Gitea
|
|
review ID 168 as `REQUEST_CHANGES`. The review was source/PR-only; the canary remained untouched.
|
|
|
|
Blocking groups:
|
|
|
|
1. class mismatch was validated after first-start mutation;
|
|
2. secure `USER.md` validation was discarded before ordinary path-following composition;
|
|
3. `existsSync()` treated a dangling destination symlink as missing, allowing counterpart partial
|
|
publication; and
|
|
4. the built-CLI/evidence chain allowed stale ignored `dist/`, cited an unshipped canary object, and
|
|
carried conflicting test totals/pane wording.
|
|
|
|
The diagnostic's defaults-only repair advice was also inaccurate for roster/class/destination
|
|
failures.
|
|
|
|
## Formal-review remediation
|
|
|
|
All four blocking groups received regressions before production changes. The RED run produced four
|
|
failures while 1,568 existing tests passed. Remediation then:
|
|
|
|
- validates canonical name and class before seeding;
|
|
- preflights destination directory entries with `lstatSync()` so target and dangling symlinks fail
|
|
before publication;
|
|
- securely reads `USER.md` through an `O_NOFOLLOW` descriptor at composition time;
|
|
- adds a Mosaic build before package Vitest and a dependency build in the clean-checkout command;
|
|
- replaces defaults-only advice with neutral named-component repair guidance; and
|
|
- reconciles shipping canary provenance, pane chronology, commands, and totals.
|
|
|
|
Remediation code review:
|
|
|
|
```bash
|
|
~/.config/mosaic/tools/codex/codex-code-review.sh --uncommitted \
|
|
-o /tmp/1264-remediation-code-review.json
|
|
```
|
|
|
|
Result: `approve`, confidence `0.88`, 6 files, no findings. Summary: the fail-closed destination
|
|
checks, class-validation order, secure composition, and build-before-Vitest path are coherent.
|
|
|
|
Remediation security review:
|
|
|
|
```bash
|
|
~/.config/mosaic/tools/codex/codex-security-review.sh --uncommitted \
|
|
-o /tmp/1264-remediation-security-review.json
|
|
```
|
|
|
|
Result: risk `none`, confidence `0.93`, 9 files, no critical/high/medium/low findings. The sandbox
|
|
could not run Vitest because Vite attempted to create a temporary config artifact on its read-only
|
|
mount (`EROFS`); executor-owned focused and full results are recorded in the QA report.
|
|
|
|
## Second exact-head review
|
|
|
|
Daphne reviewed exact head `9dc90be7e13b1cd609f6df97d43d890ef5392ca0` and filed Gitea review
|
|
ID 169 as `REQUEST_CHANGES`. Review 169 confirmed all review-168 closures, then found:
|
|
|
|
1. the new point-of-use reader was Linux-only but had been applied to every standalone USER read,
|
|
breaking supported non-fleet macOS composition; and
|
|
2. explicit blank/whitespace `MOSAIC_AGENT_CLASS` was treated as absent and could seed before
|
|
runtime, while only undefined should mean absent.
|
|
|
|
Red-first remediation preserves legacy `readOptional()` for standalone composition, keeps descriptor
|
|
no-follow consumption fleet-only, moves the replacement-symlink case under a valid fleet identity,
|
|
and rejects defined blank/whitespace classes before seeding. Three blank-class CLI cases and one
|
|
tolerant standalone composition case failed before the source change and pass after it.
|
|
|
|
Review-169 remediation code review approved at confidence `0.90` (4 files, no findings). Security
|
|
review reported risk `none` at confidence `0.90` (4 files, no findings). The review sandbox retained
|
|
its known Vite `EROFS` limitation; executor-owned tests are in the QA report.
|
|
|
|
## Remaining review gate
|
|
|
|
Daphne must re-review the next exact pushed head. This report cannot record that future verdict
|
|
without changing the reviewed head, so the authoritative terminal verdict belongs to PR #1268's
|
|
Gitea review record. Fred and goals are excluded as reviewers.
|