Skip to content

Commit 15e4b81

Browse files
ggreifKamirus
andauthored
fix(integrity): --lock update must regenerate on corrupt lockfile (#515)
Fixes #514. ## What changes for users `mops install --lock update` now reliably regenerates `mops.lock` even when the existing lockfile has a stale or corrupt per-file hash. Previously this case silently no-op'd and then exited 1 on the follow-up integrity check, so `--lock update` could not recover a broken lock — the only escape hatch was `rm mops.lock`. That hatch is no longer required. Default `mops install` (no flag) is unchanged; the fast-path shortcut still fires when the lockfile's `mopsTomlDepsHash` matches. ## Root cause `updateLockFile` short-circuits via `checkLockFileLight`, which only validates `mopsTomlDepsHash`. When mops.toml hasn't changed but the lockfile's per-file `hashes` block is corrupt, the shortcut fires → nothing is rewritten → the subsequent `checkLockFile(force)` finds the same corrupt hash and calls `process.exit(1)`. Users get "Integrity check failed" with no way out except manual deletion of `mops.lock`. ## Fix Thread the existing `force` boolean from `checkIntegrity` into `updateLockFile`. `force = !!lock` is already true exactly when the user explicitly passed `--lock update`, so: - `mops install` (no flag, outside CI) → `force = false`, keeps the fast-path - `mops install --lock update` → `force = true`, bypasses the light-check shortcut, always regenerates - `mops install --lock check` / `--lock ignore` → unchanged ## Test plan Added a regression test (`--lock update rewrites a lockfile with a corrupt file hash`) that: 1. Runs `mops install` to produce a valid lockfile. 2. Surgically replaces one per-file hash with a bogus value. 3. Runs `mops install --lock update`. 4. Asserts exit 0 and that the bogus hash is no longer in the lockfile. Verified reverting the fix (restoring `updateLockFile()` / `if (checkLockFileLight()) return`) makes the new Jest test fail with the expected "Integrity check failed" / "Mismatched hash" error. ## Related - #509 (merged as part of 2.12.2) fixed the alias-tuple count mismatch that was the common *source* of corrupt lockfiles in the wild. This PR makes `--lock update` able to heal locks that were already written in the corrupt state. --------- Co-authored-by: Kamil Listopad <listopadkamil@gmail.com> Co-authored-by: Kamil Listopad <kamil.listopad@caffeine.ai>
1 parent c58458f commit 15e4b81

4 files changed

Lines changed: 51 additions & 5 deletions

File tree

cli/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Mops CLI Changelog
22

33
## Next
4+
- Fix `mops install --lock update` silently no-op'ing on a corrupt lockfile (#515)
45
- `mops publish` no longer rejects unknown `mops.toml` sections, `package.*` keys, or `requirements.*` entries — these typo guards were the only place in the CLI that complained about unknown keys, drifted from the docs/types, and blocked publish on harmless local-only config like `[moc]`, `[canisters]`, `[build]`, and `[lint]` (#512)
56

67
## 2.12.2

cli/integrity.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export async function checkIntegrity(lock?: "check" | "update" | "ignore") {
4141
}
4242

4343
if (lock === "update") {
44-
await updateLockFile();
44+
await updateLockFile({ force });
4545
await checkLockFile(force);
4646
} else if (lock === "check") {
4747
await checkLockFile(force);
@@ -159,9 +159,13 @@ export function checkLockFileLight(): boolean {
159159
return false;
160160
}
161161

162-
export async function updateLockFile() {
162+
export async function updateLockFile({
163+
force = false,
164+
}: { force?: boolean } = {}) {
163165
// if lock file exists and mops.toml hasn't changed, don't update it
164-
if (checkLockFileLight()) {
166+
// (unless forced: `--lock update` must unconditionally regenerate so users
167+
// can recover from a corrupt lockfile without `rm mops.lock`)
168+
if (!force && checkLockFileLight()) {
165169
return;
166170
}
167171

@@ -313,6 +317,11 @@ export async function checkLockFile(force = false) {
313317
console.error(`Mismatched hash for ${fileId}`);
314318
console.error(`Locked hash: ${lockedHash}`);
315319
console.error(`Actual hash: ${localHash}`);
320+
console.error("");
321+
console.error(
322+
"If you have not modified files under .mops/, your lockfile may be stale or corrupt.",
323+
);
324+
console.error("Run `mops install --lock update` to regenerate it.");
316325
process.exit(1);
317326
}
318327
}

cli/tests/cli.test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, jest, test } from "@jest/globals";
2-
import { existsSync, rmSync } from "node:fs";
2+
import { existsSync, readFileSync, rmSync, writeFileSync } from "node:fs";
33
import path from "path";
44
import { cli } from "./helpers";
55

@@ -92,4 +92,40 @@ describe("install", () => {
9292
rmSync(path.join(cwd, ".mops"), { recursive: true, force: true });
9393
}
9494
});
95+
96+
// Regression: `install --lock update` used to early-return if mops.toml's
97+
// deps hash was unchanged, even when the lockfile's per-file hashes were
98+
// stale/corrupt. The subsequent checkLockFile would then fail and exit 1,
99+
// so `--lock update` could never recover a broken lock — the only escape
100+
// was `rm mops.lock`. See issue #514.
101+
test("--lock update rewrites a lockfile with a corrupt file hash", async () => {
102+
const cwd = path.join(import.meta.dirname, "install/success");
103+
const lockFile = path.join(cwd, "mops.lock");
104+
rmSync(lockFile, { force: true });
105+
try {
106+
const first = await cli(["install"], { cwd, env: { CI: undefined } });
107+
expect(first.exitCode).toBe(0);
108+
expect(existsSync(lockFile)).toBe(true);
109+
110+
const bad =
111+
"BAD0000000000000000000000000000000000000000000000000000000000BAD";
112+
const original = readFileSync(lockFile, "utf8");
113+
const corrupted = original.replace(
114+
/"core@1\.0\.0\/mops\.toml":\s*"[0-9a-f]{64}"/,
115+
`"core@1.0.0/mops.toml": "${bad}"`,
116+
);
117+
expect(corrupted).not.toBe(original);
118+
writeFileSync(lockFile, corrupted);
119+
120+
const result = await cli(["install", "--lock", "update"], {
121+
cwd,
122+
env: { CI: undefined },
123+
});
124+
expect(result.exitCode).toBe(0);
125+
expect(readFileSync(lockFile, "utf8")).not.toContain(bad);
126+
} finally {
127+
rmSync(lockFile, { force: true });
128+
rmSync(path.join(cwd, ".mops"), { recursive: true, force: true });
129+
}
130+
});
95131
});

docs/docs/cli/1-deps/02-mops-install.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ See [mops.lock](/mops.lock) for details on lockfile contents and when to commit
2626
What to do with the [lockfile](/mops.lock).
2727

2828
Possible values:
29-
- `update` — keep the lockfile in sync with current dependencies and verify file integrity (default)
29+
- `update` — keep the lockfile in sync with current dependencies and verify file integrity (default). Pass explicitly to force regeneration if the lockfile is stale or corrupt.
3030
- `check` — verify file integrity against an existing lockfile; fail if the lockfile is missing or out of date
3131
- `ignore` — skip the lockfile entirely
3232

0 commit comments

Comments
 (0)