diff --git a/INSTALL.md b/INSTALL.md index 2c020eee..7f8c145c 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -72,6 +72,8 @@ The compatibility wrapper does not contain its own downloader or installer. It f Self-update accepts only the latest published, non-draft, non-prerelease release from the fixed upstream repository, with a strict `vX.Y.Z` tag. It never falls back to `main`, another branch, a fork, an arbitrary URL, or GitHub's generated source archive. +If a newer release is blocked by reviewed local modifications, `--force` remains ineffective. Local regular-file drift that is already byte-identical to the verified target release can converge automatically. To intentionally replace any remaining local modifications, use `npx github-delivery update --apply --replace-local-modifications`; the verified release path backs up the complete previous installation before replacing it. The replacement flag is invalid without `--apply` and is not available to ordinary install mode. + Self-update never downgrades the installed skill. An installed skill newer than the latest published stable release is a complete no-op, including Authority reconciliation. If the skill itself is already current, `--update --apply` may still repair or update an installed/required Windows Authority host that is stale or legacy. A versioned Authority host newer than stable is never automatically downgraded. `--update` rejects `--source`, `--restore`, and `--allow-downgrade` so those separate local install/recovery controls cannot weaken release provenance. ### Verification before replacement diff --git a/README.md b/README.md index ba9f3a77..bdaeb861 100644 --- a/README.md +++ b/README.md @@ -310,7 +310,7 @@ Apply the verified plan: npx github-delivery update --apply ``` -Self-update accepts only the fixed upstream's latest stable `vX.Y.Z` GitHub Release and replaces nothing until release assets, checksums, distribution manifest, tag/source binding, constrained GitHub artifact attestation, and bounded ZIP extraction verify. Release-asset HTTP `502`, `503`, and `504` responses are retried within a fixed two-retry budget before acquisition fails; deterministic client errors remain fail-fast. Local tracked modifications block replacement even with `--force`; update does not silently downgrade an ahead install. +Self-update accepts only the fixed upstream's latest stable `vX.Y.Z` GitHub Release and replaces nothing until release assets, checksums, distribution manifest, tag/source binding, constrained GitHub artifact attestation, and bounded ZIP extraction verify. Release-asset HTTP `502`, `503`, and `504` responses are retried within a fixed two-retry budget before acquisition fails; deterministic client errors remain fail-fast. Local tracked modifications block replacement even with `--force`. Drift already identical to the same file in the verified target release can converge automatically; otherwise, after reviewing the affected paths, `update --apply --replace-local-modifications` explicitly chooses the verified release and preserves the previous installation as a backup before replacement. Update does not silently downgrade an ahead install. Exclusive skill and Windows Authority install locks record a github-delivery process identity. If that process exits hard and leaves its lock file behind, a later run reclaims the lock only when it has the exact github-delivery PID + nonce format and the recorded PID is provably gone. Live, malformed, or permission-uncertain locks stay fail-closed as `install_lock_held` rather than risking concurrent installers. diff --git a/references/update.md b/references/update.md index fa8ebaa9..75edd733 100644 --- a/references/update.md +++ b/references/update.md @@ -56,11 +56,11 @@ Transport failures, malformed metadata, redirect-policy violations, size-limit f ``` 3. Inspect the returned action: - - `update`: a strictly newer verified stable release is available and the installed tracked payload is clean. + - `update`: a strictly newer verified stable release is available and no blocking local modifications remain; the installed payload may be clean or contain only target-converged regular files that already match the verified target release. - `already_current`: the installed version equals the latest stable release. No replacement is needed. Any reported local modifications are diagnostic only because no replacement is attempted. - `already_ahead`: the installed version is newer than the latest stable release. Do not downgrade it. Any reported local modifications are diagnostic only because no replacement is attempted. - - `blocked_local_modifications`: a newer release exists, but tracked installed files differ from the installed manifest. Do not overwrite them. -4. If the action is `blocked_local_modifications`, show the affected paths and stop. `--force` does not bypass this self-update protection. + - `blocked_local_modifications`: a newer release exists and at least one local modification would be overwritten with different content. Do not overwrite it. Local regular-file drift that already matches the same path and SHA-256 in the verified target release is target-converged and does not block replacement. +4. If the action is `blocked_local_modifications`, show the affected paths and stop. `--force` does not bypass this self-update protection. Missing files, mode changes, non-regular substitutions, target-absent local files, and content that differs from the verified target remain blocking by default. After reviewing those paths, the user may explicitly choose the verified release over all remaining local modifications with `update --apply --replace-local-modifications`. That opt-in is apply-only, still uses the verified release payload, does not set installer `force`, and backs up the complete previous installation before replacement. 5. If the dry-run reports `update`, apply the same verified path: ```text diff --git a/scripts/github-delivery-cli.mjs b/scripts/github-delivery-cli.mjs index ab8ceb4b..e2e528a2 100644 --- a/scripts/github-delivery-cli.mjs +++ b/scripts/github-delivery-cli.mjs @@ -5,7 +5,7 @@ import { fileURLToPath } from "node:url"; import { parseBootstrapArgs } from "./lib/bootstrap-cli.mjs"; import { runBootstrap } from "./lib/bootstrap-command.mjs"; -export const HELP_TEXT = `GitHub Delivery\n\nUsage:\n github-delivery\n github-delivery install [--target PATH]\n github-delivery setup [--target PATH]\n github-delivery start\n github-delivery autostart [on|off|status]\n github-delivery doctor [--target PATH] [--json]\n github-delivery update [--target PATH] [--apply]\n\nBare invocation launches guided setup.\nDoctor is human-readable by default; add --json for the raw machine report.\nUpdate is dry-run by default; add --apply only after reviewing the plan.\n`; +export const HELP_TEXT = `GitHub Delivery\n\nUsage:\n github-delivery\n github-delivery install [--target PATH]\n github-delivery setup [--target PATH]\n github-delivery start\n github-delivery autostart [on|off|status]\n github-delivery doctor [--target PATH] [--json]\n github-delivery update [--target PATH] [--apply [--replace-local-modifications]]\n\nBare invocation launches guided setup.\nDoctor is human-readable by default; add --json for the raw machine report.\nUpdate is dry-run by default; add --apply only after reviewing the plan. If reviewed local changes intentionally need to be replaced by the verified release, add --replace-local-modifications together with --apply; the old installation is backed up first.\n`; function value(value, fallback = "unknown") { if (value === null || value === undefined || value === "") return fallback; diff --git a/scripts/install-skill.mjs b/scripts/install-skill.mjs index 275e0a12..0c8949f3 100755 --- a/scripts/install-skill.mjs +++ b/scripts/install-skill.mjs @@ -53,6 +53,7 @@ export function parseInstallArgs(argv, { installedRoot = resolve(import.meta.dir targetExplicit: false, allowDowngrade: false, force: false, + replaceLocalModifications: false, restore: null, codexHome, host: inferHost(codexHome), @@ -82,13 +83,20 @@ export function parseInstallArgs(argv, { installedRoot = resolve(import.meta.dir else if (arg === "--update") options.update = true; else if (arg === "--allow-downgrade") options.allowDowngrade = true; else if (arg === "--force") options.force = true; + else if (arg === "--replace-local-modifications") options.replaceLocalModifications = true; else throw new Error(`unknown argument: ${arg}`); } + if (options.replaceLocalModifications && !options.update) { + throw new Error("replace_local_modifications_update_only"); + } if (options.update) { if (options.sourceExplicit) throw new Error("update_source_conflict"); if (options.restore) throw new Error("update_restore_conflict"); if (options.allowDowngrade) throw new Error("update_allow_downgrade_forbidden"); + if (options.replaceLocalModifications && !options.apply) { + throw new Error("update_replace_local_modifications_requires_apply"); + } if (!options.targetExplicit) options.target = installedRoot; } @@ -346,7 +354,13 @@ export async function runInstallCommand(options, dependencies = {}) { const legacyMigration = candidate.plan.action === "migrate_legacy" && candidate.plan.legacyManifestless === true && candidate.plan.migrationAllowed === true; - if (!legacyMigration && (candidate.plan.action !== "update" || candidate.plan.safeToReplace !== true)) { + const localReplacementAuthorized = options.replaceLocalModifications === true + && candidate.plan.action === "blocked_local_modifications"; + if ( + !legacyMigration + && !localReplacementAuthorized + && (candidate.plan.action !== "update" || candidate.plan.safeToReplace !== true) + ) { throw new Error(`stable_release_update_blocked:${candidate.plan.action || "invalid"}`); } @@ -363,6 +377,7 @@ export async function runInstallCommand(options, dependencies = {}) { apply: true, allowDowngrade: false, force: false, + replaceLocalModifications: false, legacyManifestlessMigration: legacyMigration, }; installation = await installWithWindowsLockRecovery({ @@ -436,6 +451,7 @@ export async function runInstallCommand(options, dependencies = {}) { release: candidate.release, watchdog: installation?.watchdog || null, authorityHost, + replacedLocalModifications: localReplacementAuthorized, }; } catch (error) { if (installation?.backupPath && error && typeof error === "object") { diff --git a/scripts/lib/bootstrap-cli.mjs b/scripts/lib/bootstrap-cli.mjs index e9265965..4d0160d1 100644 --- a/scripts/lib/bootstrap-cli.mjs +++ b/scripts/lib/bootstrap-cli.mjs @@ -106,6 +106,7 @@ export function parseBootstrapArgs(argv = []) { let target = null; let help = false; let json = false; + let replaceLocalModifications = false; let autostartMode = null; let index = 0; @@ -135,6 +136,9 @@ export function parseBootstrapArgs(argv = []) { } else if (arg === "--apply") { if (command !== "update") fail("bootstrap_apply_update_only"); apply = true; + } else if (arg === "--replace-local-modifications") { + if (command !== "update") fail("bootstrap_replace_local_modifications_update_only"); + replaceLocalModifications = true; } else if (arg === "--json") { if (command !== "doctor") fail("bootstrap_json_doctor_only"); json = true; @@ -147,8 +151,13 @@ export function parseBootstrapArgs(argv = []) { } } + if (replaceLocalModifications && !apply) { + fail("bootstrap_replace_local_modifications_requires_apply"); + } + const result = { command, apply, target, help }; if (json) result.json = true; + if (replaceLocalModifications) result.replaceLocalModifications = true; if (autostartMode) result.autostartMode = autostartMode; return result; } diff --git a/scripts/lib/bootstrap-command.mjs b/scripts/lib/bootstrap-command.mjs index e298b694..cd8aa3a1 100644 --- a/scripts/lib/bootstrap-command.mjs +++ b/scripts/lib/bootstrap-command.mjs @@ -110,6 +110,7 @@ export async function runBootstrap(argv = [], dependencies = {}) { return update({ target, apply: options.apply, + ...(options.replaceLocalModifications === true ? { replaceLocalModifications: true } : {}), ...(typeof dependencies.onProgress === "function" ? { onProgress: dependencies.onProgress } : {}), }); } diff --git a/scripts/lib/bootstrap-maintenance.mjs b/scripts/lib/bootstrap-maintenance.mjs index 193ba2d3..25f0adab 100644 --- a/scripts/lib/bootstrap-maintenance.mjs +++ b/scripts/lib/bootstrap-maintenance.mjs @@ -64,6 +64,7 @@ function authorityProviderGuidance() { export async function runBootstrapUpdate({ target, apply = false, + replaceLocalModifications = false, onProgress = undefined, dependencies = {}, } = {}) { @@ -72,6 +73,7 @@ export async function runBootstrapUpdate({ const run = dependencies.runInstallCommand || runInstallCommand; const argv = ["--update", "--target", resolve(target)]; if (apply) argv.push("--apply"); + if (replaceLocalModifications) argv.push("--replace-local-modifications"); const options = parse(argv); const runDependencies = typeof onProgress === "function" ? { ...dependencies, onProgress } diff --git a/scripts/lib/release-self-update.mjs b/scripts/lib/release-self-update.mjs index 9e567bbc..fc2c95ae 100644 --- a/scripts/lib/release-self-update.mjs +++ b/scripts/lib/release-self-update.mjs @@ -499,7 +499,11 @@ export async function prepareVerifiedReleaseCandidate({ if (!payload?.verified || !payload?.releaseMetadata) { fail("stable_release_candidate_invalid"); } - const updatePlan = plan({ releases: [payload.releaseMetadata], target }); + const updatePlan = plan({ + releases: [payload.releaseMetadata], + target, + targetManifest: payload.manifest, + }); const { releaseMetadata, ...verifiedPayload } = payload; return { diff --git a/scripts/lib/stable-release-update.mjs b/scripts/lib/stable-release-update.mjs index 655a551d..eb6b5416 100644 --- a/scripts/lib/stable-release-update.mjs +++ b/scripts/lib/stable-release-update.mjs @@ -180,9 +180,71 @@ export function releaseAssetPlan(release) { }; } -export function planStableUpdate({ releases, target, installedManifest = undefined, dependencies = {} } = {}) { +function partitionLocalModificationsAgainstTarget({ + modifications, + targetManifest, + target, + dependencies = {}, +}) { + const blocking = []; + const converged = []; + if ( + !targetManifest + || targetManifest.schemaVersion !== 1 + || targetManifest.kind !== "github-delivery/distribution-manifest" + || !Array.isArray(targetManifest.files) + ) { + return { blocking: [...modifications], converged }; + } + + const targetEntries = new Map(); + for (const entry of targetManifest.files) { + const path = validateManifestPath(entry?.path); + if (typeof entry?.sha256 !== "string" || !/^[0-9a-f]{64}$/i.test(entry.sha256)) { + throw new Error("stable_release_target_manifest_invalid"); + } + targetEntries.set(path, entry); + } + + target = resolve(target); + const lstat = dependencies.lstat || lstatSync; + const readFile = dependencies.readFile || readFileSync; + const digest = dependencies.sha256 || sha256; + + for (const modification of modifications) { + if (!["changed", "local_file"].includes(modification.reason)) { + blocking.push(modification); + continue; + } + const entry = targetEntries.get(modification.path); + if (!entry) { + blocking.push(modification); + continue; + } + const path = join(target, ...modification.path.split("/")); + const stats = lstatOrMissing(lstat, path); + if (!isRegularFile(stats) || digest(readFile(path)) !== entry.sha256.toLowerCase()) { + blocking.push(modification); + continue; + } + converged.push(modification); + } + + return { blocking, converged }; +} + +export function planStableUpdate({ + releases, + target, + installedManifest = undefined, + targetManifest = undefined, + dependencies = {}, +} = {}) { const release = selectStableRelease(releases); const assets = releaseAssetPlan(release); + if (targetManifest !== undefined && targetManifest?.version !== assets.version) { + throw new Error("stable_release_target_manifest_invalid"); + } let current; let local = null; @@ -223,11 +285,20 @@ export function planStableUpdate({ releases, target, installedManifest = undefin }; } + const targetComparison = partitionLocalModificationsAgainstTarget({ + modifications: local.modifications, + targetManifest, + target, + dependencies, + }); + const safeToReplace = comparison > 0 + ? targetComparison.blocking.length === 0 + : local.clean; const action = comparison === 0 ? "already_current" : comparison < 0 ? "already_ahead" - : !local.clean + : !safeToReplace ? "blocked_local_modifications" : "update"; return { @@ -238,7 +309,9 @@ export function planStableUpdate({ releases, target, installedManifest = undefin currentVersion: current.version || null, target: resolve(target), localModifications: local.modifications, - safeToReplace: local.clean, + blockingLocalModifications: targetComparison.blocking, + targetConvergedLocalModifications: targetComparison.converged, + safeToReplace, action, assets, }; diff --git a/tests/unit/bootstrap-maintenance.test.mjs b/tests/unit/bootstrap-maintenance.test.mjs index c92eeb5b..e9755610 100644 --- a/tests/unit/bootstrap-maintenance.test.mjs +++ b/tests/unit/bootstrap-maintenance.test.mjs @@ -72,6 +72,43 @@ test("update always delegates through the installed target explicitly", async () assert.equal(result.updated, true); }); +test("update forwards explicit local-replacement authorization", async () => { + const seen = []; + await runBootstrapUpdate({ + target: TARGET, + apply: true, + replaceLocalModifications: true, + dependencies: { + parseInstallArgs(argv) { + seen.push(argv); + return { + update: true, + target: TARGET, + targetExplicit: true, + apply: true, + sourceExplicit: false, + allowDowngrade: false, + force: false, + replaceLocalModifications: true, + }; + }, + async runInstallCommand(options) { + seen.push(options); + return { action: "update", apply: true, updated: true, target: TARGET }; + }, + }, + }); + + assert.deepEqual(seen[0], [ + "--update", + "--target", + TARGET, + "--apply", + "--replace-local-modifications", + ]); + assert.equal(seen[1].replaceLocalModifications, true); +}); + test("setup fails clearly when no valid installed skill exists", async () => { await assert.rejects( runBootstrapSetup({ diff --git a/tests/unit/github-delivery-cli.test.mjs b/tests/unit/github-delivery-cli.test.mjs index 1b6555f2..186ba80c 100644 --- a/tests/unit/github-delivery-cli.test.mjs +++ b/tests/unit/github-delivery-cli.test.mjs @@ -52,6 +52,21 @@ test("parses the public npx command surface and rejects unsafe v1 options", () = target: null, help: false, }); + assert.deepEqual(parseBootstrapArgs(["update", "--apply", "--replace-local-modifications"]), { + command: "update", + apply: true, + target: null, + help: false, + replaceLocalModifications: true, + }); + assert.throws( + () => parseBootstrapArgs(["update", "--replace-local-modifications"]), + /bootstrap_replace_local_modifications_requires_apply/, + ); + assert.throws( + () => parseBootstrapArgs(["install", "--replace-local-modifications"]), + /bootstrap_replace_local_modifications_update_only/, + ); assert.deepEqual(parseBootstrapArgs(["doctor"]), { command: "doctor", apply: false, diff --git a/tests/unit/installer.test.mjs b/tests/unit/installer.test.mjs index 3595b370..faef093b 100644 --- a/tests/unit/installer.test.mjs +++ b/tests/unit/installer.test.mjs @@ -50,6 +50,27 @@ test("installer parses self-update from the running installed bundle root", () = assert.equal(options.sourceExplicit, false); }); +test("self-update local replacement requires explicit apply", () => { + const context = { installedRoot: resolve("/virtual/installed/github-delivery") }; + const options = parseInstallArgs([ + "--update", + "--apply", + "--replace-local-modifications", + ], context); + assert.equal(options.update, true); + assert.equal(options.apply, true); + assert.equal(options.replaceLocalModifications, true); + + assert.throws( + () => parseInstallArgs(["--update", "--replace-local-modifications"], context), + /update_replace_local_modifications_requires_apply/, + ); + assert.throws( + () => parseInstallArgs(["--replace-local-modifications"], context), + /replace_local_modifications_update_only/, + ); +}); + test("self-update keeps an explicit target override", () => { const installedRoot = resolve("/virtual/installed/github-delivery"); const target = resolve("/virtual/custom/github-delivery"); diff --git a/tests/unit/release-acquisition.test.mjs b/tests/unit/release-acquisition.test.mjs index 084d16bd..3238670d 100644 --- a/tests/unit/release-acquisition.test.mjs +++ b/tests/unit/release-acquisition.test.mjs @@ -143,6 +143,7 @@ test("update candidate planning reuses the shared verified acquisition result", planCalls += 1; assert.equal(options.target, target); assert.deepEqual(options.releases, [value.release]); + assert.deepEqual(options.targetManifest, value.manifest); return { schemaVersion: 1, kind: "github-delivery/stable-update-plan", diff --git a/tests/unit/release-self-update-integration.test.mjs b/tests/unit/release-self-update-integration.test.mjs index b7cd12b8..fa22501a 100644 --- a/tests/unit/release-self-update-integration.test.mjs +++ b/tests/unit/release-self-update-integration.test.mjs @@ -179,6 +179,32 @@ test("local modifications block replacement and force cannot bypass the update p assert.equal(readFileSync(join(target, "marker.txt"), "utf8"), "old\n"); })); +test("explicit local-replacement authorization reaches verified installer without force", async () => withFixture(async ({ root, target }) => { + const candidate = verifiedCandidate(root, target); + candidate.plan.action = "blocked_local_modifications"; + candidate.plan.safeToReplace = false; + candidate.plan.localModifications = [{ path: "SKILL.md", reason: "changed" }]; + + await assert.rejects( + runInstallCommand({ + update: true, + apply: true, + target, + replaceLocalModifications: true, + }, { + ...workspaceDependencies(root), + prepareVerifiedReleaseCandidate: async () => candidate, + readUserConfig: () => ({ config: { schemaVersion: 1, authorityMode: "off" } }), + installSkill(options) { + assert.equal(options.force, false); + assert.equal(options.source, candidate.source); + throw new Error("replace_local_modifications_reached_installer"); + }, + }), + /replace_local_modifications_reached_installer/, + ); +})); + test("candidate verification failure occurs before any installer mutation", async () => withFixture(async ({ root, target }) => { let installCalls = 0; await assert.rejects( diff --git a/tests/unit/stable-release-update-security.test.mjs b/tests/unit/stable-release-update-security.test.mjs index ccf086e5..888b2bfe 100644 --- a/tests/unit/stable-release-update-security.test.mjs +++ b/tests/unit/stable-release-update-security.test.mjs @@ -83,6 +83,63 @@ test("stable update never downgrades an installation ahead of the latest release assert.equal(plan.safeToReplace, true); }); +test("newer release may replace local drift that already matches the verified target manifest", () => { + const plan = planStableUpdate({ + releases: [{ + tag_name: "v0.5.0", + draft: false, + prerelease: false, + assets: requiredAssets("0.5.0"), + }], + target: "/skill", + installedManifest: manifest("0.4.0", [{ + path: "SKILL.md", + mode: "0644", + sha256: "b".repeat(64), + }]), + targetManifest: manifest("0.5.0", [{ + path: "SKILL.md", + mode: "0644", + sha256: "c".repeat(64), + }]), + dependencies: dirtyInstalledDependencies(), + }); + + assert.equal(plan.action, "update"); + assert.equal(plan.safeToReplace, true); + assert.deepEqual(plan.localModifications, [{ path: "SKILL.md", reason: "changed" }]); + assert.deepEqual(plan.blockingLocalModifications, []); + assert.deepEqual(plan.targetConvergedLocalModifications, [{ path: "SKILL.md", reason: "changed" }]); +}); + +test("newer release still blocks local drift that differs from the verified target manifest", () => { + const plan = planStableUpdate({ + releases: [{ + tag_name: "v0.5.0", + draft: false, + prerelease: false, + assets: requiredAssets("0.5.0"), + }], + target: "/skill", + installedManifest: manifest("0.4.0", [{ + path: "SKILL.md", + mode: "0644", + sha256: "b".repeat(64), + }]), + targetManifest: manifest("0.5.0", [{ + path: "SKILL.md", + mode: "0644", + sha256: "d".repeat(64), + }]), + dependencies: dirtyInstalledDependencies(), + }); + + assert.equal(plan.action, "blocked_local_modifications"); + assert.equal(plan.safeToReplace, false); + assert.deepEqual(plan.blockingLocalModifications, [{ path: "SKILL.md", reason: "changed" }]); + assert.deepEqual(plan.targetConvergedLocalModifications, []); +}); + test("current and ahead releases remain no-ops when local drift exists", () => { for (const [releaseVersion, installedVersion, expectedAction] of [ ["0.4.0", "0.4.0", "already_current"],