diff --git a/ROADMAP.md b/ROADMAP.md index 90a0ec0..546d51a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -66,8 +66,8 @@ _3.0 turned the embedded API into Packkit's public programmatic surface. Keeping ### Embedded API follow-ups (from the 3.0 review) _Non-blocking items deferred from PR #17. The blockers (symlink safety, replay mode-preservation) shipped in 3.0._ -- [ ] **Document the writer's threat model** — the writer is safe against hostile file maps and pre-existing symlinks, but not race-proof: a TOCTOU window exists between the symlink/collision preflight and the `mkdir`/`writeFile`, so another process mutating the destination concurrently could still redirect a write. The intended use is a private temp workspace, where this doesn't arise. State that boundary in [EMBEDDING.md](docs/EMBEDDING.md) rather than claiming protection we don't provide. (An `O_NOFOLLOW`/`openat`-style atomic write would close it, but isn't warranted for the current use.) -- [ ] **`driftPolicy` option for `createProjectFromDefinition`** — replay currently returns the project with an **error-severity** diagnostic when a stored `add` now collides with a generated file (it does not throw). That's deliberate — it preserves the original output — but a consumer treating any `severity: 'error'` as a hard failure would misread it. Add `createProjectFromDefinition(def, { driftPolicy: 'report' | 'error' })` so throwing is opt-in, and document that a returned project may carry error diagnostics until then. +- [x] ~~**Document the writer's threat model**~~ — **Shipped (3.1).** [EMBEDDING.md](docs/EMBEDDING.md) now states plainly what the writer protects against (hostile file maps, pre-existing symlinks) and what it doesn't (a concurrent process mutating the destination — the TOCTOU window between preflight and write), and that the intended use is a private temp workspace. +- [x] ~~**`driftPolicy` option for `createProjectFromDefinition`**~~ — **Shipped (3.1).** `createProjectFromDefinition(def, { driftPolicy: 'report' | 'error' })`. Default `'report'` returns the project with the diagnostic (unchanged); `'error'` throws on an unreconciled add-collision. Version drift alone (a warning) never throws. Documented that a returned project may carry error-severity diagnostics without throwing. ## Ideas from real-world research (2026) _From mining pain points across create-typescript-app, tsup, Changesets, create-t3-app, and current "publish an npm package" guides. Ordered by demand × fit._ diff --git a/docs/EMBEDDING.md b/docs/EMBEDDING.md index 5be8279..6409a25 100644 --- a/docs/EMBEDDING.md +++ b/docs/EMBEDDING.md @@ -123,6 +123,20 @@ const recreated = createProjectFromDefinition(definition); Loading a definition validates its schema and warns if it was created with a different Packkit version (output may differ across versions). +If a file the host originally *added* is now one that the current Packkit +generates, that's reported as an error-severity diagnostic and the stored copy +is used. To make that condition fail loudly instead: + +```js +const recreated = createProjectFromDefinition(definition, { driftPolicy: 'error' }); +// throws PackkitValidationError when an added file now collides with generated output +``` + +The default, `driftPolicy: 'report'`, returns the project with the diagnostic so +you can decide. A returned project may therefore carry error-severity +diagnostics without `createProjectFromDefinition` having thrown — inspect +`project.diagnostics` if you treat those as failures. + ## Calculate a digest For change detection or caching, a digest over the normalized config and file @@ -157,6 +171,24 @@ result.diagnostics; // per-file write outcomes An invalid path anywhere in the project makes the whole write fail before anything is written, so you never get half-escaped output. +### What the writer protects against, and what it doesn't + +The writer is safe against a **hostile file map** and **pre-existing symlinks**: +absolute paths, `..` escapes, null bytes, and paths that resolve through an +existing symbolic link (including the destination itself, and the final file +component) are all rejected before anything is written. + +It is **not** race-proof against a concurrently malicious process. There is a +window between the symlink/collision preflight and the `mkdir`/`writeFile`, so +another process mutating the destination directory *at the same time* — swapping +a checked directory for a symlink — could still redirect a write. Closing that +would require atomic `openat`/`O_NOFOLLOW`-style writes. + +For the intended use — writing a freshly generated project into a private +temporary workspace your application controls — this doesn't arise. If you're +writing into a directory another process can mutate concurrently, treat that as +outside the writer's guarantees. + ## Stable vs internal Stable, versioned API (import from `create-packkit`, `create-packkit/embedded`, diff --git a/src/embedded/index.js b/src/embedded/index.js index 6acb8bc..4e859ad 100644 --- a/src/embedded/index.js +++ b/src/embedded/index.js @@ -271,8 +271,19 @@ export function exportProjectDefinition(project) { }; } -/** Rebuild a project from a stored definition, re-applying its extensions. */ -export function createProjectFromDefinition(definition) { +/** + * Rebuild a project from a stored definition, re-applying its extensions. + * + * If a file the host originally *added* now collides with one the current + * Packkit generates, that's surfaced as an error-severity diagnostic. With + * `driftPolicy: 'error'` that condition throws instead — for a caller that + * wants an unreconciled definition to fail loudly rather than reproduce + * silently. The default, `'report'`, returns the project with the diagnostic. + */ +export function createProjectFromDefinition(definition, { driftPolicy = 'report' } = {}) { + if (!['report', 'error'].includes(driftPolicy)) { + throw new TypeError(`Unknown driftPolicy "${driftPolicy}".`); + } validateDefinition(definition); const current = packkitVersion(); const base = createProject({ preset: definition.preset, config: definition.config }); @@ -313,6 +324,14 @@ export function createProjectFromDefinition(definition) { } } + // With driftPolicy: 'error', an unreconciled add-collision fails the rebuild + // rather than reproducing silently. Version drift alone (a warning) never + // throws — only the error-severity collision does. + const fatalDrift = drift.filter((d) => d.severity === 'error'); + if (driftPolicy === 'error' && fatalDrift.length) { + throw new PackkitValidationError('The definition no longer reconciles with this Packkit version; see error.diagnostics.', fatalDrift); + } + const hasExt = Object.keys(plainFiles).length || Object.keys(ext.packageJson || {}).length; // Carry the stored modes so re-exporting the replayed project preserves the // original add/replace intent rather than re-deriving it against this base. diff --git a/test/embedded.test.js b/test/embedded.test.js index 97a7b31..314db3c 100644 --- a/test/embedded.test.js +++ b/test/embedded.test.js @@ -392,3 +392,31 @@ test('createProjectFromResolvedConfig honors a field set after resolution (repo) const project = createProjectFromResolvedConfig(config, { diagnostics }); assert.match(project.files['package.json'], /acme\/lib/); }); + +test('driftPolicy: error throws on an add-collision; report returns it as a diagnostic', () => { + const base = createProject({ preset: 'ts-lib', name: 'lib' }); + const generatedPath = Object.keys(base.files)[0]; + const definition = { + schemaVersion: SCHEMA_VERSION, + packkitVersion: '0.0.1', + preset: 'ts-lib', + config: { name: 'lib', preset: 'ts-lib' }, + extensions: { files: { [generatedPath]: { content: 'host-owned', mode: 'add' } }, packageJson: {} }, + }; + // default 'report' returns the project with an error diagnostic (no throw) + const reported = createProjectFromDefinition(definition); + assert.ok(reported.diagnostics.some((d) => d.code === 'EXTENSION_ADD_COLLIDES_WITH_NEW_BASE')); + // 'error' throws instead + assert.throws( + () => createProjectFromDefinition(definition, { driftPolicy: 'error' }), + (e) => e instanceof PackkitValidationError && e.diagnostics[0].code === 'EXTENSION_ADD_COLLIDES_WITH_NEW_BASE', + ); +}); + +test('driftPolicy: error does NOT throw on a clean replay (version drift alone is fine)', () => { + const ext = extendProject(createProject({ preset: 'ts-lib', name: 'lib' }), { files: { 'novel.txt': 'x' } }); + const def = { ...exportProjectDefinition(ext), packkitVersion: '0.0.1' }; // force a version-drift warning + const rec = createProjectFromDefinition(def, { driftPolicy: 'error' }); + assert.ok(rec.diagnostics.some((d) => d.code === 'PACKKIT_VERSION_DRIFT')); + assert.ok(rec.files['novel.txt'] === 'x'); +}); diff --git a/types/embedded.d.ts b/types/embedded.d.ts index c6c07b8..aca2524 100644 --- a/types/embedded.d.ts +++ b/types/embedded.d.ts @@ -89,6 +89,9 @@ export function resolveProjectConfig(input?: CreateProjectInput): { config: Reso export function createProjectFromResolvedConfig(config: ResolvedPackkitConfig, options?: { diagnostics?: Diagnostic[] }): GeneratedProject; export function extendProject(project: GeneratedProject, extension?: ProjectExtension): GeneratedProject; export function exportProjectDefinition(project: GeneratedProject): PackkitProjectDefinition; -export function createProjectFromDefinition(definition: PackkitProjectDefinition): GeneratedProject; +export function createProjectFromDefinition( + definition: PackkitProjectDefinition, + options?: { driftPolicy?: 'report' | 'error' }, +): GeneratedProject; export function calculateProjectDigest(project: GeneratedProject): string; export function deriveDeploymentContract(config: ResolvedPackkitConfig): DeploymentContract;