fix(db): EBUSY-safe database import on Windows (#5406) (#5507)

Closes #5406
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-29 21:21:25 -03:00
committed by GitHub
parent 2a8309ba4a
commit 73f9580c8e
3 changed files with 48 additions and 4 deletions

View File

@@ -11,6 +11,7 @@
- **services (installer):** fix `spawn EINVAL` when installing an embedded service (9Router / CLIProxy) on **Windows + Node.js 24+**. Node 24 stopped letting `child_process.execFile()` run `.cmd` batch files without a shell (nodejs/node#52554), and npm on Windows is `npm.cmd`, so `runNpm()` threw `EINVAL` the moment a user clicked **Install**. `runNpm` now enables `shell` on win32 only. To keep Hard Rule #13 intact under a shell — where the shell, not `execFile`, parses argv — the install `--prefix` (a `DATA_DIR` path that can legitimately contain spaces, e.g. `C:\Users\John Doe\.omniroute\…`) is now passed via the `npm_config_prefix` **environment variable** instead of an argv path, and the user-supplied install `version` is constrained to a dist-tag/semver shape (`SERVICE_VERSION_PATTERN`) at the route boundary so it can never carry shell metacharacters. With the prefix in the environment and the version validated, every remaining argv entry is a static flag. Regression guards: `tests/unit/services/installers/runNpm-shell-5379.test.ts` (+ existing `ninerouter.test.ts` aligned to npm's `npm_config_prefix` env). ([#5379](https://github.com/diegosouzapw/OmniRoute/issues/5379))
- **cli (serve):** restore `dist/tls-options.mjs` to the npm tarball — the opt-in native HTTPS/TLS sidecar (#5361) was copied into the staged `dist/` by the build but then **pruned** by the prepublish allowlist step, so `omniroute serve` crashed on the published 3.8.41 with `ERR_MODULE_NOT_FOUND` (`dist/server-ws.mjs` imports `./tls-options.mjs`). Added `tls-options.mjs` to `APP_STAGING_ALLOWED_EXACT_PATHS` (survives the prune) and `dist/tls-options.mjs` to `PACK_ARTIFACT_REQUIRED_PATHS` (the `check:pack-artifact` gate now fails loudly if it ever vanishes again — same guard pattern as `webdav-handler.mjs`). Regression guards in `tests/unit/pack-artifact-policy.test.ts`. ([#5452](https://github.com/diegosouzapw/OmniRoute/issues/5452))
- **dashboard:** fix the **Add Provider / onboarding wizard** button silently doing nothing. The `/dashboard/providers/new` route was a redirect stub (it bounced straight back to `/dashboard/providers`), so every "Add Provider" button and dashboard widget link opened nothing, and the fully-built `ProviderOnboardingWizard` component stayed orphaned (never rendered by any route). The route now renders the wizard directly; auth is enforced centrally by the `(dashboard)` layout, same as the sibling provider routes. Regression guard in `tests/unit/onboarding-wizard-route-5427.test.ts`. ([#5427](https://github.com/diegosouzapw/OmniRoute/issues/5427))
- **db (import):** fix `EBUSY: resource busy or locked` when importing a database on **Windows**. The import route deleted the live `storage.sqlite` + WAL/`-shm`/`-journal` sidecars with a plain `fs.unlinkSync` immediately after `resetDbInstance()`, but Windows releases the SQLite file handle asynchronously after `close()` (mmap / antivirus), so the unlink raced and threw `EBUSY`. The route now deletes via `unlinkFileWithRetry` (EBUSY/EPERM backoff) — the same helper the restore path already uses. Regression guard in `tests/unit/db-import-ebusy-5406.test.ts`. ([#5406](https://github.com/diegosouzapw/OmniRoute/issues/5406), consolidated under [#5161](https://github.com/diegosouzapw/OmniRoute/issues/5161))
---

View File

@@ -5,7 +5,12 @@ import os from "os";
import { getDbInstance, resetDbInstance, SQLITE_FILE } from "@/lib/db/core";
import { openDatabaseAsync } from "@/lib/db/adapters/driverFactory";
import type { SqliteAdapter } from "@/lib/db/adapters/types";
import { backupDbFile, getTableNamesFromAdapter, countImportedRows } from "@/lib/db/backup";
import {
backupDbFile,
getTableNamesFromAdapter,
countImportedRows,
unlinkFileWithRetry,
} from "@/lib/db/backup";
import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth";
import { getSettings } from "@/lib/db/settings";
import { setSystemPromptConfig } from "@omniroute/open-sse/services/systemPrompt.ts";
@@ -167,11 +172,12 @@ export async function POST(request: Request) {
`${SQLITE_FILE}-shm`,
`${SQLITE_FILE}-journal`,
];
// Delete with EBUSY/EPERM retry: after resetDbInstance() the OS may still
// hold the SQLite file handle for a moment (Windows mmap / antivirus), so a
// plain unlink races to EBUSY (#5406). Mirror the restore path's helper.
for (const filePath of sqliteFilesToReplace) {
if (!filePath) continue;
if (fs.existsSync(filePath)) {
fs.unlinkSync(filePath);
}
await unlinkFileWithRetry(filePath);
}
// Copy imported file over current DB

View File

@@ -0,0 +1,37 @@
import test from "node:test";
import assert from "node:assert/strict";
import { readFileSync } from "node:fs";
import { fileURLToPath } from "node:url";
import { dirname, join } from "node:path";
// Regression guard for #5406: the database-import route deleted the live
// storage.sqlite + WAL/-shm/-journal sidecars with a plain synchronous
// `fs.unlinkSync` and no retry. On Windows the OS releases the SQLite file
// handle asynchronously after `db.close()` (mmap / antivirus), so the immediate
// unlink races and throws EBUSY. The restore path already solved this with
// `unlinkFileWithRetry` (EBUSY/EPERM backoff); the import path must use the
// same helper instead of raw `fs.unlinkSync`.
const here = dirname(fileURLToPath(import.meta.url));
const repoRoot = join(here, "..", "..");
const importRoute = join(repoRoot, "src/app/api/db-backups/import/route.ts");
test("#5406: import route uses unlinkFileWithRetry (EBUSY-safe on Windows)", () => {
const src = readFileSync(importRoute, "utf8");
assert.match(
src,
/unlinkFileWithRetry/,
"import route must delete the sqlite files via unlinkFileWithRetry (EBUSY retry)"
);
});
test("#5406: import route does not raw-unlink the live sqlite files (EBUSY race)", () => {
const src = readFileSync(importRoute, "utf8");
// The buggy code deleted the sqlite + WAL sidecars with `fs.unlinkSync(filePath)`
// inside the sqliteFilesToReplace loop. Only that path races to EBUSY; the
// temp-upload cleanup (`fs.unlinkSync(tmpPath)`) is a different, unlocked file.
assert.ok(
!/fs\.unlinkSync\s*\(\s*filePath\b/.test(src),
"the sqlite-replace loop must use unlinkFileWithRetry, not raw fs.unlinkSync(filePath)"
);
});