diff --git a/changelog.d/fixes/13577-proxy-status-preserved-on-write.md b/changelog.d/fixes/13577-proxy-status-preserved-on-write.md new file mode 100644 index 0000000000..3e5dc7a37f --- /dev/null +++ b/changelog.d/fixes/13577-proxy-status-preserved-on-write.md @@ -0,0 +1 @@ +- **fix(proxies):** a subscription refresh, a bulk re-import or an API update that omits the status no longer turns a disabled proxy back on, and a refresh no longer rewrites a manual proxy that shares a subscription node's address ([#13577](https://github.com/diegosouzapw/OmniRoute/pull/13577)) — thanks @maxmad64bis diff --git a/src/app/(dashboard)/dashboard/settings/components/ProxyRegistryManager.tsx b/src/app/(dashboard)/dashboard/settings/components/ProxyRegistryManager.tsx index 990d17988d..3af5b91361 100644 --- a/src/app/(dashboard)/dashboard/settings/components/ProxyRegistryManager.tsx +++ b/src/app/(dashboard)/dashboard/settings/components/ProxyRegistryManager.tsx @@ -581,7 +581,7 @@ import { password: entry.password || undefined, region: entry.region || null, notes: entry.notes || null, - status: entry.status as "active" | "inactive", + status: entry.status as "active" | "inactive" | undefined, })), }; @@ -1413,13 +1413,13 @@ import { {entry.port} {entry.username || "—"} {entry.region || "—"} - + - {entry.status === "active" ? t("statusActive") : t("statusInactive")} + {entry.status === "active" && t("statusActive")} + {entry.status === "inactive" && t("statusInactive")} + {!entry.status && "—"} diff --git a/src/app/(dashboard)/dashboard/settings/components/parseBulkProxyImport.ts b/src/app/(dashboard)/dashboard/settings/components/parseBulkProxyImport.ts index cbd70440a3..61b7b27ed0 100644 --- a/src/app/(dashboard)/dashboard/settings/components/parseBulkProxyImport.ts +++ b/src/app/(dashboard)/dashboard/settings/components/parseBulkProxyImport.ts @@ -28,7 +28,8 @@ export type ParsedProxyEntry = { password: string; type: string; region: string; - status: string; + /** Absent when the line carries no status: the import then leaves the stored one alone. */ + status?: string; notes: string; }; @@ -90,7 +91,6 @@ function pushShorthandEntry( password, type: normalizedType, region: "", - status: "active", notes: "", }); return true; @@ -236,8 +236,8 @@ export function parseBulkImportText(text: string): { errors.push({ line: lineNum, reason: "bulkImportErrorInvalidType" }); continue; } - const normalizedStatus = (status || "active").toLowerCase(); - if (!VALID_PROXY_STATUSES[normalizedStatus]) { + const normalizedStatus = status ? status.toLowerCase() : undefined; + if (normalizedStatus !== undefined && !VALID_PROXY_STATUSES[normalizedStatus]) { errors.push({ line: lineNum, reason: "bulkImportErrorInvalidStatus" }); continue; } @@ -250,7 +250,7 @@ export function parseBulkImportText(text: string): { password: password || "", type: normalizedType, region: region || "", - status: normalizedStatus, + ...(normalizedStatus ? { status: normalizedStatus } : {}), notes: notes || "", }); continue; diff --git a/src/lib/db/proxies.ts b/src/lib/db/proxies.ts index 016f0d3bb5..1b0e562476 100755 --- a/src/lib/db/proxies.ts +++ b/src/lib/db/proxies.ts @@ -29,6 +29,7 @@ import { } from "./proxies/mappers"; import { isGlobalProxyEnabled, PROXY_ALIVE_PREDICATE } from "./proxies/guards"; import { bumpProxyRegistryGeneration } from "./proxies/registryGeneration"; +import { isProxyRegistryStatus } from "@/shared/constants/proxyRegistryStatus"; export { hasBlockingProxyAssignment, hasBlockingProxyAssignmentForProvider, @@ -334,22 +335,48 @@ export async function createProxy(payload: ProxyPayload) { * * #7703: password is mutable and must not be part of the identity key. Including * it caused password-only credential rotations to create duplicate entries. + * + * On an existing row the status is written only when the payload carries a valid one, + * so a write that omits it never revives a proxy the operator or auto-disable turned off. + * + * `claimOwnership: false` is reserved for subscription sync: a matched row whose + * subscription_id differs from the payload's (manual rows included) is not written + * at all and the call returns `action: "skipped"`. The sync always sends its subscriptionId: + * a caller that omits it would find manual rows (null === null) counted as owned. */ -export async function upsertProxy(payload: ProxyPayload): Promise<{ - proxy: ProxyRegistryRecord | null; - action: "created" | "updated"; -}> { +export async function upsertProxy( + payload: ProxyPayload +): Promise<{ proxy: ProxyRegistryRecord | null; action: "created" | "updated" }>; +export async function upsertProxy( + payload: ProxyPayload, + options: { claimOwnership?: boolean } +): Promise<{ proxy: ProxyRegistryRecord | null; action: "created" | "updated" | "skipped" }>; +export async function upsertProxy( + payload: ProxyPayload, + options: { claimOwnership?: boolean } = {} +): Promise<{ proxy: ProxyRegistryRecord | null; action: "created" | "updated" | "skipped" }> { const db = getDbInstance(); const host = (payload.host || "").trim(); const port = Number(payload.port); const username = (payload.username || "").trim(); const existing = db - .prepare("SELECT id FROM proxy_registry WHERE host = ? AND port = ? AND username = ? LIMIT 1") - .get(host, port, username) as { id?: string } | undefined; + .prepare( + "SELECT id, subscription_id FROM proxy_registry WHERE host = ? AND port = ? AND username = ? LIMIT 1" + ) + .get(host, port, username) as { id?: string; subscription_id?: string | null } | undefined; if (existing?.id) { - const updated = await updateProxy(existing.id, payload); + const claimOwnership = options.claimOwnership ?? true; + const ownerChanged = (existing.subscription_id ?? null) !== (payload.subscriptionId ?? null); + if (!claimOwnership && ownerChanged) { + return { proxy: null, action: "skipped" }; + } + const { status, ...rest } = payload; + const changes: Partial = isProxyRegistryStatus(status) + ? { ...rest, status } + : rest; + const updated = await updateProxy(existing.id, changes); return { proxy: updated, action: "updated" }; } @@ -358,6 +385,8 @@ export async function upsertProxy(payload: ProxyPayload): Promise<{ } export async function updateProxy(id: string, payload: Partial) { + // No status filtering here: callers own the status they send. Writes that must + // preserve the stored status filter it in upsertProxy before calling this. const db = getDbInstance(); const existing = await getProxyById(id, { includeSecrets: true }); if (!existing) return null; diff --git a/src/lib/proxySubscription/subscriptionService.ts b/src/lib/proxySubscription/subscriptionService.ts index 60d0ce7158..cf92db7718 100644 --- a/src/lib/proxySubscription/subscriptionService.ts +++ b/src/lib/proxySubscription/subscriptionService.ts @@ -26,6 +26,7 @@ import { addProxiesToScopePool, bumpProxyRegistryGeneration, deleteProxyById, + updateProxy, upsertProxy, } from "../db/proxies"; import { bumpProxyConfigGeneration } from "../db/settings"; @@ -384,6 +385,23 @@ async function fetchSubscriptionContent(url: string): Promise { }); } +/** + * Keep a synced node only when this subscription owns its registry row. A row created + * by hand, or owned by another subscription, comes back as "skipped" and stays out of + * this pool. An owned row that pool validation flagged `error` is healed, since the feed + * just listed it again; `inactive` and `dead` are operator or health decisions and stay. + */ +async function keepOwnedSyncedRow( + upserted: Awaited>, + keptIds: string[] +): Promise { + if (upserted.action === "skipped" || !upserted.proxy?.id) return; + keptIds.push(upserted.proxy.id); + if (upserted.proxy.status === "error") { + await updateProxy(upserted.proxy.id, { status: "active" }); + } +} + /** Fetch + parse + sync nodes into proxy_registry, then (if enabled) (re)bind. */ async function syncSubscriptionUnsafe(id: string): Promise { const sub = await getSubscriptionById(id); @@ -416,18 +434,21 @@ async function syncSubscriptionUnsafe(id: string): Promise { try { // Directly-usable nodes → upsert into the registry as a pool. for (const node of parsed.nodes) { - const upserted = await upsertProxy({ - name: node.name || `${sub.name} (${node.host}:${node.port})`, - type: node.type, - host: node.host, - port: node.port, - username: node.username, - password: node.password, - source: "subscription", - subscriptionId: id, - status: "active", - }); - if (upserted.proxy?.id) keptIds.push(upserted.proxy.id); + // No status: a refresh must not revive a node the operator or auto-disable turned off. + const upserted = await upsertProxy( + { + name: node.name || `${sub.name} (${node.host}:${node.port})`, + type: node.type, + host: node.host, + port: node.port, + username: node.username, + password: node.password, + source: "subscription", + subscriptionId: id, + }, + { claimOwnership: false } + ); + await keepOwnedSyncedRow(upserted, keptIds); } // needsCore nodes → bind the operator-supplied local core endpoint (single). @@ -436,18 +457,20 @@ async function syncSubscriptionUnsafe(id: string): Promise { try { const coreUrl = new URL(sub.localCoreEndpoint); const coreType = coreUrl.protocol === "https:" ? "https" : coreUrl.protocol === "socks5:" ? "socks5" : "http"; - const upserted = await upsertProxy({ - name: `${sub.name} (local core)`, - type: coreType, - host: coreUrl.hostname, - port: Number(coreUrl.port) || (coreType === "https" ? 443 : 8080), - username: coreUrl.username ? decodeURIComponent(coreUrl.username) : undefined, - password: coreUrl.password ? decodeURIComponent(coreUrl.password) : undefined, - source: "subscription", - subscriptionId: id, - status: "active", - }); - if (upserted.proxy?.id) keptIds.push(upserted.proxy.id); + const upserted = await upsertProxy( + { + name: `${sub.name} (local core)`, + type: coreType, + host: coreUrl.hostname, + port: Number(coreUrl.port) || (coreType === "https" ? 443 : 8080), + username: coreUrl.username ? decodeURIComponent(coreUrl.username) : undefined, + password: coreUrl.password ? decodeURIComponent(coreUrl.password) : undefined, + source: "subscription", + subscriptionId: id, + }, + { claimOwnership: false } + ); + await keepOwnedSyncedRow(upserted, keptIds); } catch { warning = subscriptionErrorCode("LOCAL_CORE_ENDPOINT_INVALID"); } diff --git a/src/shared/constants/proxyRegistryStatus.ts b/src/shared/constants/proxyRegistryStatus.ts new file mode 100644 index 0000000000..55841ccacf --- /dev/null +++ b/src/shared/constants/proxyRegistryStatus.ts @@ -0,0 +1,13 @@ +/** + * Statuses a caller may write on a proxy registry row. `error` is deliberately not + * listed: only pool validation sets it, and no import or update may send it. + */ +export const PROXY_REGISTRY_STATUS_VALUES = ["active", "inactive", "dead"] as const; + +export type ProxyRegistryStatus = (typeof PROXY_REGISTRY_STATUS_VALUES)[number]; + +export function isProxyRegistryStatus(value: unknown): value is ProxyRegistryStatus { + return ( + typeof value === "string" && (PROXY_REGISTRY_STATUS_VALUES as readonly string[]).includes(value) + ); +} diff --git a/src/shared/validation/schemas/proxy.ts b/src/shared/validation/schemas/proxy.ts index 8f2322d63c..9fc0391652 100644 --- a/src/shared/validation/schemas/proxy.ts +++ b/src/shared/validation/schemas/proxy.ts @@ -14,6 +14,7 @@ import { isForbiddenCustomHeaderName, } from "@/shared/constants/upstreamHeaders"; import { MAX_TIMER_TIMEOUT_MS } from "@/shared/utils/runtimeTimeouts"; +import { PROXY_REGISTRY_STATUS_VALUES } from "@/shared/constants/proxyRegistryStatus"; export const proxyConfigSchema = z .object({ @@ -116,7 +117,9 @@ export const proxyRegistryFieldsSchema = z password: z.string().optional(), region: z.string().trim().max(64).nullable().optional(), notes: z.string().trim().max(1000).nullable().optional(), - status: z.enum(["active", "inactive", "dead"]).optional().default("active"), + // No default: zod 4 applies it under .partial() too, which rewrote the stored status + // on every update or import that omitted it. New rows still start active in the DB. + status: z.enum(PROXY_REGISTRY_STATUS_VALUES).optional(), source: z .enum([ "manual", diff --git a/tests/unit/proxy-registry-manager.test.ts b/tests/unit/proxy-registry-manager.test.ts index 9d2ec9798c..e5936ce21e 100644 --- a/tests/unit/proxy-registry-manager.test.ts +++ b/tests/unit/proxy-registry-manager.test.ts @@ -14,7 +14,7 @@ test("auth-less host:port produces socks5 entry with generated name (default typ assert.equal(e.type, "socks5"); assert.equal(e.username, ""); assert.equal(e.password, ""); - assert.equal(e.status, "active"); + assert.equal("status" in e, false); assert.match(e.name, /127\.0\.0\.1:7897/); }); @@ -241,7 +241,7 @@ test("pipe-delimited minimal NAME|HOST|PORT defaults type to socks5", () => { assert.equal(errors.length, 0); assert.equal(entries.length, 1); assert.equal(entries[0].type, "socks5"); - assert.equal(entries[0].status, "active"); + assert.equal("status" in entries[0], false); }); test("pipe-delimited missing NAME produces error", () => { @@ -338,3 +338,65 @@ test("bare text with no colons or pipes produces error", () => { assert.equal(errors.length, 1); assert.equal(errors[0].reason, "bulkImportErrorMissingHost"); }); + +// ── Status is written only when the line carries one ────────────────────────── + +test("pipe-delimited line with an empty STATUS column has no status key", () => { + const { entries, errors } = parseBulkImportText("p|10.0.0.3|1080|||socks5|US||note"); + assert.equal(errors.length, 0); + assert.equal("status" in entries[0], false); +}); + +test("pipe-delimited line with STATUS=inactive keeps it", () => { + const { entries, errors } = parseBulkImportText("p|10.0.0.4|1080|||socks5|US|inactive|note"); + assert.equal(errors.length, 0); + assert.equal(entries[0].status, "inactive"); +}); + +test("pipe-delimited line with an unknown STATUS is rejected", () => { + const { entries, errors } = parseBulkImportText("p|10.0.0.5|1080|||socks5|US|foo|note"); + assert.equal(entries.length, 0); + assert.equal(errors[0].reason, "bulkImportErrorInvalidStatus"); +}); + +// ── Dashboard send payload omits a missing status ─────────────────────────── +// Mirrors the item mapping in ProxyRegistryManager (bulk import send): a line +// without a status must reach the API without a status key, so the stored one +// is left alone. JSON.stringify drops undefined values, which is what makes an +// `entry.status as ... | undefined` mapping safe to send as-is. + +function buildSendBody(entries: Array<{ [key: string]: unknown }>) { + const payload = { + items: entries.map((entry) => ({ + name: entry.name, + type: entry.type, + host: entry.host, + port: entry.port, + username: (entry.username as string) || undefined, + password: (entry.password as string) || undefined, + region: (entry.region as string) || null, + notes: (entry.notes as string) || null, + status: entry.status as "active" | "inactive" | undefined, + })), + }; + return JSON.parse(JSON.stringify(payload)) as { + items: Array<{ [key: string]: unknown }>; + }; +} + +test("send payload for a line without a status carries no status key", () => { + const { entries, errors } = parseBulkImportText("p|10.0.0.6|1080"); + assert.equal(errors.length, 0); + assert.equal("status" in entries[0], false); + const body = buildSendBody(entries); + assert.equal("status" in body.items[0], false); +}); + +test("send payload for a line with STATUS=inactive keeps inactive", () => { + const { entries, errors } = parseBulkImportText( + "p|10.0.0.7|1080|||socks5|US|inactive|note" + ); + assert.equal(errors.length, 0); + const body = buildSendBody(entries); + assert.equal(body.items[0].status, "inactive"); +}); diff --git a/tests/unit/proxy-registry-status-schema.test.ts b/tests/unit/proxy-registry-status-schema.test.ts new file mode 100644 index 0000000000..8437301df7 --- /dev/null +++ b/tests/unit/proxy-registry-status-schema.test.ts @@ -0,0 +1,95 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +// zod 4 applies .default() even under .partial(), so a default status on the shared +// proxy field schema rewrote the stored status on every update or import that simply +// omitted it. These tests pin "a status is written only when the caller sends one". + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-proxy-status-schema-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = "test-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const proxiesDb = await import("../../src/lib/db/proxies.ts"); +const schemas = await import("../../src/shared/validation/schemas/proxy.ts"); +const statuses = await import("../../src/shared/constants/proxyRegistryStatus.ts"); +const { handleProxyUpdate } = await import("../../src/lib/api/proxyRegistryRouteHandlers.ts"); + +function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +test("update schema does not invent a status the client did not send", () => { + const parsed = schemas.updateProxyRegistrySchema.parse({ id: "row-1", name: "renamed" }); + assert.equal("status" in parsed, false); +}); + +test("bulk import schema does not invent a status the client did not send", () => { + const parsed = schemas.bulkImportProxiesSchema.parse({ + items: [{ name: "n", host: "proxy.example.com", port: 8080 }], + }); + assert.equal("status" in parsed.items[0], false); +}); + +test("an explicit status is still validated against the enumeration", () => { + const bad = schemas.bulkImportProxiesSchema.safeParse({ + items: [{ name: "n", host: "proxy.example.com", port: 8080, status: "bogus" }], + }); + assert.equal(bad.success, false); + const good = schemas.updateProxyRegistrySchema.parse({ id: "row-2", status: "inactive" }); + assert.equal(good.status, "inactive"); +}); + +test("isProxyRegistryStatus accepts only the enumerated statuses", () => { + for (const value of ["active", "inactive", "dead"]) { + assert.equal(statuses.isProxyRegistryStatus(value), true, value); + } + for (const value of ["", "error", "ACTIVE", undefined, null, 1]) { + assert.equal(statuses.isProxyRegistryStatus(value), false, String(value)); + } +}); + +test("renaming a dead proxy through the update handler keeps it dead", async () => { + resetStorage(); + const created = await proxiesDb.createProxy({ + name: "before", + type: "http", + host: "10.1.0.1", + port: 8080, + }); + await proxiesDb.updateProxy(created.id, { status: "dead" }); + + const response = await handleProxyUpdate( + new Request("http://localhost/api/v1/management/proxies", { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ id: created.id, name: "after" }), + }) + ); + + assert.equal(response.status, 200); + const row = await proxiesDb.getProxyById(created.id); + assert.equal(row?.name, "after"); + assert.equal(row?.status, "dead"); +}); + +test("creating a proxy without a status still creates it active", async () => { + resetStorage(); + const created = await proxiesDb.createProxy({ + name: "fresh", + type: "http", + host: "10.1.0.2", + port: 8080, + }); + assert.equal((await proxiesDb.getProxyById(created.id))?.status, "active"); +}); diff --git a/tests/unit/proxy-subscription-sync-ownership.test.ts b/tests/unit/proxy-subscription-sync-ownership.test.ts new file mode 100644 index 0000000000..36bcbed97b --- /dev/null +++ b/tests/unit/proxy-subscription-sync-ownership.test.ts @@ -0,0 +1,210 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import http from "node:http"; + +// A subscription refresh must not revive a node the operator or auto-disable turned off, +// and must never write a registry row it does not own (a manual proxy or another +// subscription's node sharing the same host/port/username). + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-sub-sync-ownership-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const proxies = await import("../../src/lib/db/proxies.ts"); +const sub = await import("../../src/lib/proxySubscription/index.ts"); + +function reset() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +function feed(name: string, port: number) { + return [ + "proxies:", + ` - name: ${name}`, + " type: http", + " server: 127.0.0.1", + ` port: ${port}`, + ].join("\n"); +} + +function startFeedServer(initialBody: string): Promise<{ + url: string; + setBody: (body: string) => void; + close: () => Promise; +}> { + let body = initialBody; + return new Promise((resolve) => { + const srv = http.createServer((_req, res) => { + res.writeHead(200, { "Content-Type": "text/plain" }); + res.end(body); + }); + srv.listen(0, "127.0.0.1", () => { + const addr = srv.address(); + if (!addr || typeof addr === "string") throw new Error("no addr"); + resolve({ + url: `http://127.0.0.1:${addr.port}/list`, + setBody: (next) => { + body = next; + }, + close: () => new Promise((r) => srv.close(() => r())), + }); + }); + }); +} + +function insertSubscription(id: string, url: string) { + const now = new Date().toISOString(); + core + .getDbInstance() + .prepare( + `INSERT INTO proxy_subscriptions + (id, name, url, enabled, mode, rule_providers, update_interval_minutes, status, created_at, updated_at) + VALUES (?, ?, ?, 1, 'global', NULL, 60, 'empty', ?, ?)` + ) + .run(id, `sub-${id}`, url, now, now); +} + +function rowIdFor(port: number) { + const row = core + .getDbInstance() + .prepare("SELECT id FROM proxy_registry WHERE host = '127.0.0.1' AND port = ?") + .get(port) as { id: string } | undefined; + assert.ok(row, `expected a registry row on port ${port}`); + return row.id; +} + +test("a refresh keeps an owned node dead and still refreshes its name", async () => { + reset(); + const server = await startFeedServer(feed("node-v1", 18101)); + try { + insertSubscription("s1", server.url); + await sub.syncSubscription("s1"); + const id = rowIdFor(18101); + await proxies.updateProxy(id, { status: "dead" }); + + server.setBody(feed("node-v2", 18101)); + await sub.syncSubscription("s1"); + + const row = await proxies.getProxyById(id); + assert.equal(row?.status, "dead"); + assert.equal(row?.name, "node-v2"); + } finally { + await server.close(); + } +}); + +test("a refresh heals an owned node that pool validation flagged error", async () => { + reset(); + const server = await startFeedServer(feed("node", 18102)); + try { + insertSubscription("s1", server.url); + await sub.syncSubscription("s1"); + const id = rowIdFor(18102); + await proxies.updateProxy(id, { status: "error" }); + + await sub.syncSubscription("s1"); + + assert.equal((await proxies.getProxyById(id))?.status, "active"); + } finally { + await server.close(); + } +}); + +test("a new node is created active and owned by the subscription", async () => { + reset(); + const server = await startFeedServer(feed("node", 18103)); + try { + insertSubscription("s1", server.url); + const result = await sub.syncSubscription("s1"); + const row = await proxies.getProxyById(rowIdFor(18103)); + assert.equal(row?.status, "active"); + assert.equal(row?.source, "subscription"); + assert.equal(row?.subscriptionId, "s1"); + assert.equal(result.boundProxies, 1); + } finally { + await server.close(); + } +}); + +test("a manual proxy with the same tuple is not written, pooled or removed", async () => { + reset(); + const manual = await proxies.createProxy({ + name: "manual", + type: "https", + host: "127.0.0.1", + port: 18104, + password: "manual-pass", + status: "inactive", + }); + const before = await proxies.getProxyById(manual.id, { includeSecrets: true }); + const server = await startFeedServer(feed("feed-node", 18104)); + try { + insertSubscription("s1", server.url); + const result = await sub.syncSubscription("s1"); + + assert.deepEqual(await proxies.getProxyById(manual.id, { includeSecrets: true }), before); + assert.equal(result.boundProxies, 0); + const pooled = core + .getDbInstance() + .prepare("SELECT COUNT(*) AS n FROM proxy_assignments WHERE proxy_id = ?") + .get(manual.id) as { n: number }; + assert.equal(pooled.n, 0); + } finally { + await server.close(); + } +}); + +test("two subscriptions listing the same tuple: the row stays with the first one", async () => { + reset(); + const serverA = await startFeedServer(feed("from-a", 18105)); + const serverB = await startFeedServer(feed("from-b", 18105)); + try { + insertSubscription("sa", serverA.url); + insertSubscription("sb", serverB.url); + await sub.syncSubscription("sa"); + const id = rowIdFor(18105); + const before = await proxies.getProxyById(id, { includeSecrets: true }); + + const resultB = await sub.syncSubscription("sb"); + + assert.deepEqual(await proxies.getProxyById(id, { includeSecrets: true }), before); + assert.equal(before?.subscriptionId, "sa"); + assert.equal(resultB.boundProxies, 0); + } finally { + await serverA.close(); + await serverB.close(); + } +}); + +test("a refresh keeps the dead local core row a needs-core feed binds", async () => { + reset(); + const server = await startFeedServer("ss://YWVzLTI1Ni1nY206cGFzcw@203.0.113.9:8388#ss-node"); + try { + insertSubscription("s1", server.url); + core + .getDbInstance() + .prepare("UPDATE proxy_subscriptions SET local_core_endpoint = ? WHERE id = ?") + .run("socks5://127.0.0.1:18106", "s1"); + const first = await sub.syncSubscription("s1"); + const id = rowIdFor(18106); + assert.equal(first.boundProxies, 1); + assert.equal((await proxies.getProxyById(id))?.subscriptionId, "s1"); + await proxies.updateProxy(id, { status: "dead" }); + + await sub.syncSubscription("s1"); + + assert.equal((await proxies.getProxyById(id))?.status, "dead"); + } finally { + await server.close(); + } +}); diff --git a/tests/unit/proxy-upsert-preserves-status.test.ts b/tests/unit/proxy-upsert-preserves-status.test.ts new file mode 100644 index 0000000000..55c1bd4f45 --- /dev/null +++ b/tests/unit/proxy-upsert-preserves-status.test.ts @@ -0,0 +1,141 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +// upsertProxy is the single writer shared by bulk import and subscription sync. A write +// that carries no valid status must leave the stored one alone, and the sync must be +// able to refuse writing a row it does not own. + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-proxy-upsert-status-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const proxiesDb = await import("../../src/lib/db/proxies.ts"); + +function resetStorage() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.beforeEach(() => { + resetStorage(); +}); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +const tuple = { type: "http", host: "10.2.0.1", port: 8080, username: "u" }; + +async function seed(status?: string) { + const created = await proxiesDb.upsertProxy({ name: "seed", ...tuple, password: "old" }); + assert.equal(created.action, "created"); + const id = created.proxy!.id; + if (status) await proxiesDb.updateProxy(id, { status }); + return id; +} + +test("a new row without a status is created active", async () => { + const id = await seed(); + assert.equal((await proxiesDb.getProxyById(id))?.status, "active"); +}); + +test("re-importing a disabled proxy without a status keeps it disabled", async () => { + const id = await seed("inactive"); + const again = await proxiesDb.upsertProxy({ name: "seed", ...tuple, password: "new" }); + assert.equal(again.action, "updated"); + const row = await proxiesDb.getProxyById(id, { includeSecrets: true }); + assert.equal(row?.status, "inactive"); + assert.equal(row?.password, "new"); +}); + +test("an explicit status on re-import is applied", async () => { + const id = await seed(); + await proxiesDb.upsertProxy({ name: "seed", ...tuple, status: "inactive" }); + assert.equal((await proxiesDb.getProxyById(id))?.status, "inactive"); +}); + +test("an empty, unknown or undefined status on an existing row is ignored", async () => { + const id = await seed("dead"); + await proxiesDb.upsertProxy({ name: "seed", ...tuple, status: "" }); + await proxiesDb.upsertProxy({ name: "seed", ...tuple, status: "bogus" }); + await proxiesDb.upsertProxy({ name: "seed", ...tuple, status: undefined }); + assert.equal((await proxiesDb.getProxyById(id))?.status, "dead"); +}); + +test("a row flagged error stays in error on re-import", async () => { + const id = await seed("error"); + await proxiesDb.upsertProxy({ name: "seed", ...tuple }); + assert.equal((await proxiesDb.getProxyById(id))?.status, "error"); +}); + +test("an explicit source on import is still applied by default", async () => { + const id = await seed(); + await proxiesDb.upsertProxy({ name: "seed", ...tuple, source: "oneproxy" }); + assert.equal((await proxiesDb.getProxyById(id))?.source, "oneproxy"); +}); + +test("claimOwnership false leaves a manual row completely untouched", async () => { + const id = await seed("dead"); + const before = await proxiesDb.getProxyById(id, { includeSecrets: true }); + + const result = await proxiesDb.upsertProxy( + { + name: "feed node", + ...tuple, + type: "socks5", + password: "feed-pass", + source: "subscription", + subscriptionId: "sub-b", + }, + { claimOwnership: false } + ); + + assert.equal(result.action, "skipped"); + assert.equal(result.proxy, null); + assert.deepEqual(await proxiesDb.getProxyById(id, { includeSecrets: true }), before); +}); + +test("claimOwnership false leaves another subscription's row untouched", async () => { + const created = await proxiesDb.upsertProxy({ + name: "owned by a", + ...tuple, + source: "subscription", + subscriptionId: "sub-a", + }); + const id = created.proxy!.id; + const before = await proxiesDb.getProxyById(id, { includeSecrets: true }); + + const result = await proxiesDb.upsertProxy( + { name: "claimed by b", ...tuple, source: "subscription", subscriptionId: "sub-b" }, + { claimOwnership: false } + ); + + assert.equal(result.action, "skipped"); + assert.deepEqual(await proxiesDb.getProxyById(id, { includeSecrets: true }), before); +}); + +test("claimOwnership false still updates a row the same subscription owns", async () => { + const created = await proxiesDb.upsertProxy({ + name: "old name", + ...tuple, + source: "subscription", + subscriptionId: "sub-a", + }); + const id = created.proxy!.id; + await proxiesDb.updateProxy(id, { status: "dead" }); + + const result = await proxiesDb.upsertProxy( + { name: "new name", ...tuple, source: "subscription", subscriptionId: "sub-a" }, + { claimOwnership: false } + ); + + assert.equal(result.action, "updated"); + const row = await proxiesDb.getProxyById(id); + assert.equal(row?.name, "new name"); + assert.equal(row?.status, "dead"); +});