diff --git a/.env.example b/.env.example index 9acbfb6e33..84d7d9d49a 100644 --- a/.env.example +++ b/.env.example @@ -140,8 +140,12 @@ GITTENSORY_REVIEW_DRAFT=false # GITHUB_APP_PRIVATE_KEY= # GITHUB_APP_SLUG= # GITTENSOR_REGISTRY_URL= -# GITTENSORY_API_TOKEN= -# GITTENSORY_MCP_TOKEN= +# LOOPOVER_API_TOKEN= # #4774: the loopover rebrand's name for this token; the legacy +# # GITTENSORY_API_TOKEN below still works unchanged (dual-read; +# # this new name wins if BOTH are set) +# GITTENSORY_API_TOKEN= # legacy name, still read +# LOOPOVER_MCP_TOKEN= # #4774: ditto — legacy GITTENSORY_MCP_TOKEN below still works +# GITTENSORY_MCP_TOKEN= # legacy name, still read # INTERNAL_JOB_TOKEN= # --- Optional (capability-gated; degrade safely when absent) ----------------- @@ -521,8 +525,9 @@ REDIS_URL=redis://redis:6379 # REQUIRED for the self-host review # # never prompts, diffs, tokens, or bodies. # SENTRY_RELEASE= # custom images only: set this ONLY when you uploaded source maps for # # the exact built bundle under this exact release id. Future official -# # images bake GITTENSORY_VERSION=gittensory-selfhost@, so do -# # not override SENTRY_RELEASE for those images. +# # images bake GITTENSORY_VERSION=gittensory-selfhost@ (the +# # app also reads LOOPOVER_VERSION -- #4774 dual-read, new name wins +# # if both are set), so do not override SENTRY_RELEASE for those images. # OTEL_METRIC_EXPORT_INTERVAL=10000 # ms between metric exports (default 10s here; CLI default is 60s) # OTEL_EXPORTER_OTLP_ENDPOINT=http://otel-collector:4318 # override only for an external collector # OTEL_SERVICE_NAME=gittensory-selfhost @@ -555,7 +560,8 @@ REDIS_URL=redis://redis:6379 # REQUIRED for the self-host review # # Sentry error tracking. OFF when SENTRY_DSN is unset. Official self-host release images bake # GITTENSORY_VERSION=gittensory-selfhost@; initSentry uses that as the release id unless -# SENTRY_RELEASE is set explicitly (useful for custom/local images). +# SENTRY_RELEASE is set explicitly (useful for custom/local images). A LOOPOVER_VERSION override is also +# read (#4774 dual-read: set LOOPOVER_VERSION to take priority over the image-baked legacy name). # SENTRY_DSN= # SENTRY_DSN_FILE= # optional mounted secret file; existing *_FILE loader reads it # SENTRY_ENVIRONMENT=selfhost @@ -649,7 +655,9 @@ REDIS_URL=redis://redis:6379 # REQUIRED for the self-host review # Codex (ChatGPT subscription) reviewer is fail-closed by default for self-host PR review: `codex exec` stores its # OAuth credential in auth.json on the same filesystem that prompt-influenced reviews can read. Isolated maintainer # deployments can opt in explicitly after mounting auth at /data/codex (the image exposes it as ~/.codex). -# GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER=1 +# LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER=1 # #4774: preferred name; legacy GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER=1 +# # below still works unchanged (dual-read; this new name wins if both are set) +# GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER=1 # legacy name, still read # Do NOT set CODEX_HOME for the app container; the provider rejects it so credentials are not advertised in env. # CODEX_AI_MODEL=gpt-5.5 # omit to let the Codex account default choose; set explicitly for repeatable reviews # CODEX_AI_EFFORT=medium # low | medium | high | xhigh. `max` is accepted and maps to xhigh. diff --git a/.env.selfhost.example b/.env.selfhost.example index 3690df5c7d..2b01917618 100644 --- a/.env.selfhost.example +++ b/.env.selfhost.example @@ -32,8 +32,13 @@ GITTENSOR_REGISTRY_URL=https://example.invalid/registry.json # value, so copy-pasting the same string into more than one of these will also fail preflight. Or # skip uncommenting these entirely and write the value into secrets/.txt instead (see above). # GITHUB_WEBHOOK_SECRET= # the sole HMAC key GitHub webhook deliveries are verified against -# GITTENSORY_API_TOKEN= # server-to-server API bearer token — bypasses per-repo write checks -# GITTENSORY_MCP_TOKEN= # shared MCP bearer token +# LOOPOVER_API_TOKEN= # server-to-server API bearer token — bypasses per-repo write checks +# # (#4774: preferred name; legacy GITTENSORY_API_TOKEN below still works +# # unchanged — dual-read, this new name wins if both are set) +# GITTENSORY_API_TOKEN= # legacy name, still read +# LOOPOVER_MCP_TOKEN= # shared MCP bearer token (#4774: ditto — legacy GITTENSORY_MCP_TOKEN +# # below still works unchanged) +# GITTENSORY_MCP_TOKEN= # legacy name, still read # INTERNAL_JOB_TOKEN= # gates internal-only routes # REQUIRED for the first-run /setup wizard that creates the GitHub App manifest, AND required ongoing: @@ -88,13 +93,14 @@ GITTENSORY_REVIEW_ENRICHMENT=false # filesystem prompt-influenced reviews can read) -- the explicit opt-in below is required, and you # must mount your Codex auth at /data/codex (the image exposes it as ~/.codex). # AI_PROVIDER=codex -# GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER=1 +# LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER=1 # #4774: preferred name; legacy GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER=1 +# # still works unchanged (dual-read, this new name wins if both are set) # --- Both, synthesized into one decision -------------------------------------- # AI_PROVIDER=claude-code,codex # AI_COMBINE=synthesis # CLAUDE_CODE_OAUTH_TOKEN= -# GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER=1 +# LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER=1 # #4774: same dual-read as above # ============================================================================= # 5. Telemetry — informational only, no action needed diff --git a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts index c41a9338b8..c7f61b9bb0 100644 --- a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts +++ b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts @@ -193,6 +193,10 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ name: "GITTENSORY_ENABLE_PAGERDUTY", firstReference: "src/services/notify-pagerduty.ts", }, + { + name: "GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER", + firstReference: "src/selfhost/ai.ts", + }, { name: "GITTENSORY_REPO_CONFIG_DIR", firstReference: "src/server.ts", @@ -205,6 +209,22 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ name: "HOME", firstReference: "src/selfhost/ai.ts", }, + { + name: "LOOPOVER_ENABLE_PAGERDUTY", + firstReference: "src/services/notify-pagerduty.ts", + }, + { + name: "LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER", + firstReference: "src/selfhost/ai.ts", + }, + { + name: "LOOPOVER_REPO_CONFIG_DIR", + firstReference: "src/server.ts", + }, + { + name: "LOOPOVER_VERSION", + firstReference: "src/selfhost/otel.ts", + }, { name: "MAINTENANCE_ADMISSION_DEFER_MS", firstReference: "src/selfhost/maintenance-admission.ts", @@ -529,9 +549,14 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| `GITHUB_INSTALLATION_CONCURRENCY_ENABLED` | `src/selfhost/installation-concurrency-admission.ts` |", "| `GITHUB_INSTALLATION_CONCURRENCY_LIMIT` | `src/selfhost/installation-concurrency-admission.ts` |", "| `GITTENSORY_ENABLE_PAGERDUTY` | `src/services/notify-pagerduty.ts` |", + "| `GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER` | `src/selfhost/ai.ts` |", "| `GITTENSORY_REPO_CONFIG_DIR` | `src/server.ts` |", "| `GITTENSORY_VERSION` | `src/selfhost/otel.ts` |", "| `HOME` | `src/selfhost/ai.ts` |", + "| `LOOPOVER_ENABLE_PAGERDUTY` | `src/services/notify-pagerduty.ts` |", + "| `LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER` | `src/selfhost/ai.ts` |", + "| `LOOPOVER_REPO_CONFIG_DIR` | `src/server.ts` |", + "| `LOOPOVER_VERSION` | `src/selfhost/otel.ts` |", "| `MAINTENANCE_ADMISSION_DEFER_MS` | `src/selfhost/maintenance-admission.ts` |", "| `MAINTENANCE_ADMISSION_DRAIN_AGE_MS` | `src/selfhost/maintenance-admission.ts` |", "| `MAINTENANCE_ADMISSION_ENABLED` | `src/selfhost/maintenance-admission.ts` |", diff --git a/scripts/gen-selfhost-env-reference.mjs b/scripts/gen-selfhost-env-reference.mjs index ab8148f521..f3bd223410 100644 --- a/scripts/gen-selfhost-env-reference.mjs +++ b/scripts/gen-selfhost-env-reference.mjs @@ -77,6 +77,10 @@ function collectEnvReads(source, fileName) { } } else if (ts.isCallExpression(node) && isStaticEnvHelperCall(node)) { addRead(node.arguments[1].text); + } else if (ts.isCallExpression(node) && isDualPrefixEnvHelperCall(node)) { + const suffix = node.arguments[1].text; + addRead(`LOOPOVER_${suffix}`); + addRead(`GITTENSORY_${suffix}`); } else if (ts.isCallExpression(node) && isProcessEnvNameHelperCall(node)) { addRead(node.arguments[0].text); } else if (ts.isCallExpression(node) && isEnvNameLiteralArgHelperCall(node)) { @@ -99,6 +103,25 @@ function isStaticEnvHelperCall(node) { ); } +// #4774 dual-read (src/utils/env.ts): dualPrefixEnvString/dualPrefixEnvFlag/dualPrefixEnvStrictFlag each read +// BOTH `LOOPOVER_` and the legacy `GITTENSORY_` under the hood via template-literal key +// concatenation this AST scanner can't see into otherwise -- without this, converting a call site from a +// literal `env.GITTENSORY_X` read to `dualPrefixEnvString(env, "X")` would silently DROP that var from the +// generated reference instead of adding its new LOOPOVER_ alias. Recognized by function name only (no +// isEnvContainer check on arg[0]): real call sites pass several different container variable names for the +// same underlying env object (`env`, `parent`, `parentEnv`, `rawEnv`, a `(... as unknown as Record<...>)` +// cast, or `process.env`), so requiring isEnvContainer here would silently miss most of them. +const DUAL_PREFIX_ENV_HELPERS = new Set(["dualPrefixEnvString", "dualPrefixEnvFlag", "dualPrefixEnvStrictFlag"]); + +function isDualPrefixEnvHelperCall(node) { + return ( + ts.isIdentifier(node.expression) && + DUAL_PREFIX_ENV_HELPERS.has(node.expression.text) && + node.arguments.length >= 2 && + ts.isStringLiteralLike(node.arguments[1]) + ); +} + // Some self-host helpers read `process.env` internally by name rather than taking an env container argument -- // e.g. `parsePositiveIntEnv("QUEUE_CONCURRENCY", { min: 1, fallback: 4 })`. Recognized separately from // isStaticEnvHelperCall above (envString) because these take the var NAME as arg[0], not arg[1] after a diff --git a/src/api/routes.ts b/src/api/routes.ts index b2538cbc44..8f7f3eec2f 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -299,6 +299,7 @@ import type { RepositorySettings, } from "../types"; import { errorMessage, nowIso } from "../utils/json"; +import { dualPrefixEnvString } from "../utils/env"; import { queueDeadLetterPageFromBinding, queueDeleteDeadLetterJobViaBinding, @@ -2083,7 +2084,8 @@ export function createApp() { ...(installationHealth.some((health) => health.status !== "healthy") ? ["One or more GitHub App installations need attention."] : []), ]; const upstreamLaunchBlocking = upstreamDrift.status === "unavailable" || upstreamDrift.highestSeverity === "high" || upstreamDrift.highestSeverity === "blocking"; - const ready = Boolean(snapshot) && Boolean(c.env.INTERNAL_JOB_TOKEN) && Boolean(c.env.GITTENSORY_API_TOKEN); + // #4774 dual-read: LOOPOVER_API_TOKEN counts as configured too, same precedence as authenticatePrivateToken. + const ready = Boolean(snapshot) && Boolean(c.env.INTERNAL_JOB_TOKEN) && Boolean(dualPrefixEnvString(c.env as unknown as Record, "API_TOKEN")); const readyForPublicReview = snapshot ? snapshot.repoCount > 0 && ready && @@ -2148,8 +2150,9 @@ export function createApp() { githubAppPrivateKey: Boolean(c.env.GITHUB_APP_PRIVATE_KEY), githubWebhookSecret: Boolean(c.env.GITHUB_WEBHOOK_SECRET), githubPublicToken: Boolean(c.env.GITHUB_PUBLIC_TOKEN), - apiToken: Boolean(c.env.GITTENSORY_API_TOKEN), - mcpToken: Boolean(c.env.GITTENSORY_MCP_TOKEN), + // #4774 dual-read: reflects whichever of LOOPOVER_/GITTENSORY_ actually resolves (see dualPrefixEnvString). + apiToken: Boolean(dualPrefixEnvString(c.env as unknown as Record, "API_TOKEN")), + mcpToken: Boolean(dualPrefixEnvString(c.env as unknown as Record, "MCP_TOKEN")), internalJobToken: Boolean(c.env.INTERNAL_JOB_TOKEN), }, warnings, diff --git a/src/auth/security.ts b/src/auth/security.ts index cdb7cf0840..6b0d762ccc 100644 --- a/src/auth/security.ts +++ b/src/auth/security.ts @@ -7,6 +7,7 @@ import { } from "../db/repositories"; import type { AuthSessionRecord, JsonValue } from "../types"; import { nowIso } from "../utils/json"; +import { dualPrefixEnvString } from "../utils/env"; export type AuthIdentity = | { kind: "static"; actor: "api" | "mcp" | "internal" } @@ -102,8 +103,12 @@ export function createOpaqueToken(prefix = "gts"): string { export async function authenticatePrivateToken(env: Env, token: string | undefined): Promise { if (!token) return null; - if (await timingSafeEqual(token, env.GITTENSORY_API_TOKEN)) return { kind: "static", actor: "api" }; - if (await timingSafeEqual(token, env.GITTENSORY_MCP_TOKEN)) return { kind: "static", actor: "mcp" }; + // #4774 dual-read: LOOPOVER_API_TOKEN / LOOPOVER_MCP_TOKEN win over their legacy GITTENSORY_ names when + // both are set -- this is the real auth gate, so it must change in lockstep with preflight.ts's strength + // check, or a self-hoster who only set the new name would pass preflight but fail every authenticated call. + const rawEnv = env as unknown as Record; + if (await timingSafeEqual(token, dualPrefixEnvString(rawEnv, "API_TOKEN"))) return { kind: "static", actor: "api" }; + if (await timingSafeEqual(token, dualPrefixEnvString(rawEnv, "MCP_TOKEN"))) return { kind: "static", actor: "mcp" }; return authenticateSessionToken(env, token); } diff --git a/src/env.d.ts b/src/env.d.ts index dfc1cf1efd..91afc01ace 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -215,8 +215,11 @@ declare global { /** Self-host container-private per-repo config dir. When set, the focus-manifest loader reads * `{dir}/{owner}__{repo}.{yml,yaml,json}` INSTEAD of the public `.gittensory.yml`, so review policy (gate, * autonomy, labels, model/effort) is set privately and contributors can't read or game it. Unset ⇒ public - * fetch (cloud, or a self-host without the dir, is byte-identical to before). */ + * fetch (cloud, or a self-host without the dir, is byte-identical to before). + * #4774 dual-read: LOOPOVER_REPO_CONFIG_DIR below wins over this legacy name when both are set. */ GITTENSORY_REPO_CONFIG_DIR?: string; + /** #4774: LOOPOVER_ companion for GITTENSORY_REPO_CONFIG_DIR above — wins when both are set. */ + LOOPOVER_REPO_CONFIG_DIR?: string; GITTENSORY_AUTO_FILE_DRIFT_ISSUES?: string; GITTENSORY_DRIFT_ISSUE_REPO?: string; GITTENSORY_DRIFT_ISSUE_TOKEN?: string; @@ -236,8 +239,11 @@ declare global { * (merged/closed/manual) for ANY repo. Sibling of DISCORD_WEBHOOK_URL; set either, both, or neither. */ SLACK_WEBHOOK_URL?: string; /** Experimental (#4937/#5007): enables PagerDuty incident paging from src/services/notify-pagerduty.ts. - * Default OFF — unset/false keeps every export there a no-op. Truthy: `/^(1|true|yes|on)$/i`. */ + * Default OFF — unset/false keeps every export there a no-op. Truthy: `/^(1|true|yes|on)$/i`. + * #4774 dual-read: LOOPOVER_ENABLE_PAGERDUTY below wins over this legacy name when both are set. */ GITTENSORY_ENABLE_PAGERDUTY?: string; + /** #4774: LOOPOVER_ companion for GITTENSORY_ENABLE_PAGERDUTY above — wins when both are set. */ + LOOPOVER_ENABLE_PAGERDUTY?: string; /** Global fallback PagerDuty Events API v2 routing key (32 lowercase hex chars) for any repo not present in * PAGERDUTY_REPO_ROUTING_KEYS (a JSON `{repoFullName: routingKey}` map, read directly off the env — same * deliberately-untyped pattern as DISCORD_REPO_WEBHOOKS, since a free-form per-repo map isn't worth a @@ -258,8 +264,17 @@ declare global { PAGERDUTY_COOLDOWN_MINUTES?: string; GITTENSORY_CONTRIBUTOR_ISSUE_TOKEN?: string; PRODUCT_USAGE_HASH_SALT?: string; - GITTENSORY_API_TOKEN: string; - GITTENSORY_MCP_TOKEN: string; + /** Server-to-server API bearer token — bypasses per-repo write checks (src/auth/security.ts). + * #4774 dual-read: no longer always-present at the type level, since either this OR LOOPOVER_API_TOKEN + * below may supply the effective value (LOOPOVER_ wins when both are set) — see dualPrefixEnvString. */ + GITTENSORY_API_TOKEN?: string; + /** #4774: LOOPOVER_ companion for GITTENSORY_API_TOKEN above — wins when both are set. */ + LOOPOVER_API_TOKEN?: string; + /** Shared MCP bearer token (src/auth/security.ts). #4774 dual-read: see GITTENSORY_API_TOKEN's note above — + * either this OR LOOPOVER_MCP_TOKEN below may supply the effective value. */ + GITTENSORY_MCP_TOKEN?: string; + /** #4774: LOOPOVER_ companion for GITTENSORY_MCP_TOKEN above — wins when both are set. */ + LOOPOVER_MCP_TOKEN?: string; INTERNAL_JOB_TOKEN: string; /** Repos the shared GITTENSORY_MCP_TOKEN may propose/decide/manage actions on (comma/whitespace `owner/repo` * list, or `*`/`all` for every repo). Unset ⇒ none — GITTENSORY_MCP_TOKEN is a shared, end-user-obtainable diff --git a/src/selfhost/ai.ts b/src/selfhost/ai.ts index fbf238ba95..446bd635e0 100644 --- a/src/selfhost/ai.ts +++ b/src/selfhost/ai.ts @@ -12,6 +12,7 @@ export { assertNoLegacySharedAiEnv } from "./ai-config"; import { incr, observe } from "./metrics"; import { withReviewSpan } from "./tracing"; import { delimiter } from "node:path"; +import { dualPrefixEnvStrictFlag } from "../utils/env"; interface AiRunOptions { // Content is a plain string for every message any pre-#4111 caller ever built (byte-identical). A @@ -476,7 +477,9 @@ function assertCodexCredentialIsolation(parent: Record; @@ -109,7 +111,9 @@ export function codexAuthReadinessProbe( checkAuthFile: (env: Record) => Promise = defaultCodexAuthFileCheck, cacheMs = 30_000, ): ReadinessProbe | null { - if (parentEnv.GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER !== "1") return null; + // #4774 dual-read: LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER wins over the legacy name when both are set; strict + // "1"-only, matching this flag's intentionally narrow (non-loose-truthy) opt-in convention. + if (!dualPrefixEnvStrictFlag(parentEnv, "ENABLE_UNSAFE_CODEX_REVIEWER")) return null; let cached: boolean | undefined; let cachedUntil = 0; let inFlight: Promise | undefined; diff --git a/src/selfhost/otel.ts b/src/selfhost/otel.ts index 61de0261f3..d2f6c3b2a6 100644 --- a/src/selfhost/otel.ts +++ b/src/selfhost/otel.ts @@ -1,6 +1,7 @@ import { AsyncLocalStorage } from "node:async_hooks"; import type { Attributes, Context, ContextManager, TextMapPropagator, Tracer } from "@opentelemetry/api"; import type { ReadableSpan, Sampler, Span, SpanProcessor } from "@opentelemetry/sdk-trace-base"; +import { dualPrefixEnvString } from "../utils/env"; type OtelApi = typeof import("@opentelemetry/api"); type OtelSdk = typeof import("@opentelemetry/sdk-trace-node"); @@ -59,7 +60,8 @@ function serviceAttributes(env: NodeJS.ProcessEnv): Attributes { "service.name": nonBlank(env.OTEL_SERVICE_NAME) ?? "gittensory-selfhost", "deployment.environment.name": nonBlank(env.OTEL_SERVICE_ENVIRONMENT) ?? nonBlank(env.SENTRY_ENVIRONMENT) ?? "selfhost", }; - const version = nonBlank(env.GITTENSORY_VERSION) ?? nonBlank(env.SENTRY_RELEASE); + // #4774 dual-read: LOOPOVER_VERSION wins over the legacy GITTENSORY_VERSION when both are set. + const version = dualPrefixEnvString(env, "VERSION") ?? nonBlank(env.SENTRY_RELEASE); if (version) attrs["service.version"] = version; return attrs; } diff --git a/src/selfhost/preflight.ts b/src/selfhost/preflight.ts index 991e55c4dc..828f3ebb0b 100644 --- a/src/selfhost/preflight.ts +++ b/src/selfhost/preflight.ts @@ -1,4 +1,5 @@ import { createPrivateKey } from "node:crypto"; +import { dualPrefixEnvString } from "../utils/env"; export type SelfHostPreflightProblem = { var: string; @@ -114,13 +115,22 @@ function criticalSecretProblem(name: string, value: string): string | null { return null; } +// #4774 dual-read: a CRITICAL_SECRET_VARS entry prefixed GITTENSORY_ (currently API_TOKEN, MCP_TOKEN) also +// accepts its LOOPOVER_ companion, new name winning when both are set -- same precedence as every other +// dual-read var. The reported problem is still keyed by the legacy name in CRITICAL_SECRET_VARS either way, +// matching every existing preflight test/message; only the VALUE being judged changes. +function resolvedCriticalSecretValue(name: (typeof CRITICAL_SECRET_VARS)[number], env: SelfHostPreflightEnv): string | undefined { + if (!name.startsWith("GITTENSORY_")) return nonBlank(env[name]); + return dualPrefixEnvString(env, name.slice("GITTENSORY_".length)); +} + function checkCriticalSecrets( problems: SelfHostPreflightProblem[], env: SelfHostPreflightEnv, ): void { const seenValues = new Map(); // value -> first var name that used it for (const name of CRITICAL_SECRET_VARS) { - const value = nonBlank(env[name]); + const value = resolvedCriticalSecretValue(name, env); if (!value) continue; // presence is each caller's own concern; this only judges strength when SET const problem = criticalSecretProblem(name, value); if (problem) { diff --git a/src/selfhost/sentry.ts b/src/selfhost/sentry.ts index a9c8e1e870..88bb44a59f 100644 --- a/src/selfhost/sentry.ts +++ b/src/selfhost/sentry.ts @@ -14,6 +14,7 @@ import { } from "./otel"; import { hashedInstallationIdWith } from "./review-tracing"; import { queueDeadLetterReviveIntervalMs } from "./queue-common"; +import { dualPrefixEnvString } from "../utils/env"; type SentryNs = typeof import("@sentry/node"); type SentryClient = NonNullable>; @@ -185,11 +186,13 @@ function setOtelTraceScope(scope: SentryScope): void { scope.setContext("otel", { ...trace }); } -/** Resolve the Sentry release id from explicit override first, then the image-baked self-host version. */ +/** Resolve the Sentry release id from explicit override first, then the image-baked self-host version + * (checked under either LOOPOVER_VERSION or the legacy GITTENSORY_VERSION -- #4774 dual-read, new name + * wins when both are set). */ export function resolveSentryRelease( env: NodeJS.ProcessEnv, ): string | undefined { - return nonBlank(env.SENTRY_RELEASE) ?? nonBlank(env.GITTENSORY_VERSION); + return nonBlank(env.SENTRY_RELEASE) ?? dualPrefixEnvString(env, "VERSION"); } export function resolveSentryTracesSampleRate( diff --git a/src/server.ts b/src/server.ts index 9065e985ff..44ef9e3bd1 100644 --- a/src/server.ts +++ b/src/server.ts @@ -107,6 +107,7 @@ import { import { probeReesSecretAtStartup } from "./review/enrichment-wire"; import { sampleRecentDeadLetters } from "./selfhost/dlq-recent"; import type { JobMessage } from "./types"; +import { dualPrefixEnvString } from "./utils/env"; interface Backend { @@ -284,15 +285,14 @@ async function main(): Promise { // manifest loader prefers a mounted `{owner}__{repo}.yml`, deep-merged over an optional root `.gittensory.yml` // global default, over the public `.gittensory.yml` (review policy stays private; see // config/examples/README.md). Unset dir ⇒ null reader ⇒ unchanged public-fetch behavior. - setLocalManifestReader( - makeLocalManifestReader(process.env.GITTENSORY_REPO_CONFIG_DIR), - ); + // #4774 dual-read: LOOPOVER_REPO_CONFIG_DIR wins over the legacy GITTENSORY_REPO_CONFIG_DIR when both are + // set; resolved once here so every reader below and the boot-time log line agree on the same value. + const repoConfigDir = dualPrefixEnvString(process.env, "REPO_CONFIG_DIR"); + setLocalManifestReader(makeLocalManifestReader(repoConfigDir)); // Per-repo review CONTEXT (#review-skills): the same config dir also holds `/review/AGENTS.md` // (or legacy `/review/CLAUDE.md`) + skills/*.md, injected into the reviewer prompt so reviews follow each // repo's conventions. Unset dir ⇒ null reader ⇒ no change. - setLocalReviewContextReader( - makeLocalReviewContextReader(process.env.GITTENSORY_REPO_CONFIG_DIR), - ); + setLocalReviewContextReader(makeLocalReviewContextReader(repoConfigDir)); // Boot-time visibility (config-drift guardrail): state which config dir is actually in effect, unconditionally // -- neither reader above logs anything, so an operator previously had no way to confirm from the logs alone // which directory (if any) was live, which is exactly the ambiguity that let a stale, no-longer-mounted config @@ -301,8 +301,8 @@ async function main(): Promise { console.log( JSON.stringify({ event: "selfhost_config_dir", - configured: Boolean(process.env.GITTENSORY_REPO_CONFIG_DIR?.trim()), - dir: process.env.GITTENSORY_REPO_CONFIG_DIR?.trim() || null, + configured: Boolean(repoConfigDir), + dir: repoConfigDir ?? null, }), ); // Error tracking (#1468): opt-in via SENTRY_DSN — a complete no-op when unset. When on, capture uncaught crashes diff --git a/src/services/notify-discord.ts b/src/services/notify-discord.ts index 6b69e88153..faf3478a20 100644 --- a/src/services/notify-discord.ts +++ b/src/services/notify-discord.ts @@ -23,6 +23,8 @@ function isValidDiscordWebhook(url: string): boolean { // fall back to the global webhook: falling back posts repo A's disposition into repo B's channel. Generic // self-hosters should prefer DISCORD_REPO_WEBHOOKS for per-repo routing, or DISCORD_WEBHOOK_URL for one shared // channel across unmapped repos. +// #4774 dual-read: any name here starting with GITTENSORY_ also has a LOOPOVER_ companion (e.g. +// LOOPOVER_DISCORD_WEBHOOK) resolved below, new name winning when both are set -- see resolveDiscordWebhook. const WEBHOOK_SECRET_BY_REPO: Record = { "jsonbored/gittensory": "GITTENSORY_DISCORD_WEBHOOK", "jsonbored/metagraphed": "METAGRAPHED_DISCORD_WEBHOOK", @@ -69,7 +71,10 @@ export function resolveDiscordWebhook(env: Env, repoFullName: string): DiscordWe const name = WEBHOOK_SECRET_BY_REPO[repoKey]; if (name) { - const mapped = envString(env, name); + // #4774 dual-read: e.g. LOOPOVER_DISCORD_WEBHOOK wins over the legacy GITTENSORY_DISCORD_WEBHOOK when + // both are set; a secret name outside the GITTENSORY_ family (METAGRAPHED_/AWESOME_) has no companion. + const loopoverName = name.startsWith("GITTENSORY_") ? name.replace("GITTENSORY_", "LOOPOVER_") : null; + const mapped = (loopoverName ? envString(env, loopoverName) : undefined) ?? envString(env, name); return mapped && isValidDiscordWebhook(mapped) ? { status: "configured", url: mapped, source: "legacy_repo_secret" } : { status: "disabled", reason: mapped ? "invalid_repo_webhook" : "missing_repo_webhook" }; } diff --git a/src/services/notify-pagerduty.ts b/src/services/notify-pagerduty.ts index 7d1ff49b9e..4f1dd1541b 100644 --- a/src/services/notify-pagerduty.ts +++ b/src/services/notify-pagerduty.ts @@ -1,5 +1,6 @@ import { countRecentAuditEventsForActorAndTarget, recordAuditEvent } from "../db/repositories"; import { errorMessage } from "../utils/json"; +import { dualPrefixEnvFlag } from "../utils/env"; // PagerDuty Events API v2 (https://developer.pagerduty.com/docs/events-api-v2/overview/). Experimental, // default-OFF (GITTENSORY_ENABLE_PAGERDUTY) — a self-host operator opts in per #4937's paging epic. @@ -26,9 +27,13 @@ const DEFAULT_MIN_SEVERITY: PagerDutySeverity = "error"; const DEFAULT_COOLDOWN_MINUTES = 60; /** True when the experimental PagerDuty integration is enabled. Flag-OFF (default) → every export below is a - * no-op. Truthy follows the codebase convention (`/^(1|true|yes|on)$/i`, same as isOpsEnabled/isSafetyEnabled). */ -export function isPagerDutyEnabled(env: { GITTENSORY_ENABLE_PAGERDUTY?: string | undefined }): boolean { - return /^(1|true|yes|on)$/i.test(env.GITTENSORY_ENABLE_PAGERDUTY ?? ""); + * no-op. Truthy follows the codebase convention (`/^(1|true|yes|on)$/i`, same as isOpsEnabled/isSafetyEnabled). + * #4774 dual-read: LOOPOVER_ENABLE_PAGERDUTY wins over the legacy GITTENSORY_ENABLE_PAGERDUTY when both are set. */ +export function isPagerDutyEnabled(env: { + GITTENSORY_ENABLE_PAGERDUTY?: string | undefined; + LOOPOVER_ENABLE_PAGERDUTY?: string | undefined; +}): boolean { + return dualPrefixEnvFlag(env as unknown as Record, "ENABLE_PAGERDUTY"); } function envString(env: Env, name: string): string | undefined { @@ -64,7 +69,11 @@ export type PagerDutyRoutingResolution = /** Resolve the PagerDuty routing key for `repoFullName`: per-repo map entry, else the global fallback, else * disabled. Mirrors {@link resolveDiscordWebhook}'s exact precedence and shape. */ export function resolvePagerDutyRoutingKey(env: Env, repoFullName: string): PagerDutyRoutingResolution { - if (!isPagerDutyEnabled(env as unknown as { GITTENSORY_ENABLE_PAGERDUTY?: string | undefined })) { + if ( + !isPagerDutyEnabled( + env as unknown as { GITTENSORY_ENABLE_PAGERDUTY?: string | undefined; LOOPOVER_ENABLE_PAGERDUTY?: string | undefined }, + ) + ) { return { status: "disabled", reason: "flag_off" }; } const repoKey = repoFullName.toLowerCase(); diff --git a/src/utils/env.ts b/src/utils/env.ts new file mode 100644 index 0000000000..8f7964a2e1 --- /dev/null +++ b/src/utils/env.ts @@ -0,0 +1,47 @@ +// gittensory -> loopover rebrand, self-host env var prefix rename (#4774): every operator-facing env var +// historically prefixed `GITTENSORY_` gets a `LOOPOVER_` companion name. This is a DUAL-READ addition, +// never a cutover -- an existing self-hoster's `.env` that sets only the legacy `GITTENSORY_` name must +// keep working completely unchanged. +// +// Precedence: the new `LOOPOVER_` name wins when BOTH are set. This mirrors the repo's existing +// "explicit override first" precedent for a materially identical two-source resolution -- +// `resolveSentryRelease` (src/selfhost/sentry.ts): `nonBlank(env.SENTRY_RELEASE) ?? nonBlank(env.GITTENSORY_VERSION)`, +// where the more specific/recently-set override always wins over the broader/older fallback. Applied +// here: an operator who has started migrating to LOOPOVER_ sees their new value take effect immediately, +// while an untouched .env stays byte-identical to today on the legacy GITTENSORY_ name. +// +// Lives outside `src/selfhost/**` (unlike its siblings `nonBlank`/`envString`, which are duplicated +// per-file there) because two of the seven affected vars (GITTENSORY_API_TOKEN, GITTENSORY_MCP_TOKEN) are +// actually consumed by core auth code (src/auth/security.ts) and the public API (src/api/routes.ts), not +// just src/selfhost/**. + +function nonBlank(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + return trimmed ? trimmed : undefined; +} + +/** Read `LOOPOVER_`, falling back to the legacy `GITTENSORY_` when the new name is unset + * or blank (see precedence note above). `env` is any plain string-keyed record: `process.env`, or a + * Worker `Env` widened at the call site (`env as unknown as Record`) since + * `Env` itself carries non-string bindings that don't satisfy a `string | undefined` index signature. */ +export function dualPrefixEnvString( + env: Record, + suffix: string, +): string | undefined { + return nonBlank(env[`LOOPOVER_${suffix}`]) ?? nonBlank(env[`GITTENSORY_${suffix}`]); +} + +/** Boolean-flag sibling of {@link dualPrefixEnvString}, reusing the codebase-wide loose truthy-string + * convention (`/^(1|true|yes|on)$/i`, same as `isPagerDutyEnabled`/`isOpsEnabled`/`isSafetyEnabled`). + * Use for a flag that was already loose-truthy under its `GITTENSORY_` name, e.g. + * `GITTENSORY_ENABLE_PAGERDUTY` -> `LOOPOVER_ENABLE_PAGERDUTY`. */ +export function dualPrefixEnvFlag(env: Record, suffix: string): boolean { + return /^(1|true|yes|on)$/i.test(dualPrefixEnvString(env, suffix) ?? ""); +} + +/** Strict `"1"`-only sibling of {@link dualPrefixEnvString}, for a flag that intentionally does NOT use + * the loose truthy convention above -- e.g. `GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER` -> a deliberately + * narrow "unsafe opt-in" flag whose accepted values must not silently broaden while renaming it. */ +export function dualPrefixEnvStrictFlag(env: Record, suffix: string): boolean { + return dualPrefixEnvString(env, suffix) === "1"; +} diff --git a/test/unit/auth.test.ts b/test/unit/auth.test.ts index db16524f9d..c31c15b7c9 100644 --- a/test/unit/auth.test.ts +++ b/test/unit/auth.test.ts @@ -33,6 +33,33 @@ describe("private-beta auth and rate limiting", () => { await expect(authenticatePrivateToken(env, malformed.token)).resolves.toBeNull(); }); + // #4774 dual-read: GITTENSORY_API_TOKEN/GITTENSORY_MCP_TOKEN get a LOOPOVER_ companion here at the REAL + // auth gate — this must change in lockstep with preflight.ts's strength check, or a self-hoster who only + // set the new name would pass preflight but fail every authenticated call. + it("authenticates static tokens supplied via the NEW LOOPOVER_ names alone (legacy names unset)", async () => { + const env = createTestEnv({ LOOPOVER_API_TOKEN: "new-api-token", LOOPOVER_MCP_TOKEN: "new-mcp-token" }); + delete (env as Partial).GITTENSORY_API_TOKEN; + delete (env as Partial).GITTENSORY_MCP_TOKEN; + await expect(authenticatePrivateToken(env, "new-api-token")).resolves.toMatchObject({ kind: "static", actor: "api" }); + await expect(authenticatePrivateToken(env, "new-mcp-token")).resolves.toMatchObject({ kind: "static", actor: "mcp" }); + // The old (now-unset) names must no longer authenticate. + await expect(authenticatePrivateToken(env, "test-api-token")).resolves.toBeNull(); + }); + + it("still authenticates static tokens supplied via the legacy GITTENSORY_ names alone — an untouched .env keeps working unchanged", async () => { + const env = createTestEnv(); + await expect(authenticatePrivateToken(env, "test-api-token")).resolves.toMatchObject({ kind: "static", actor: "api" }); + await expect(authenticatePrivateToken(env, "test-mcp-token")).resolves.toMatchObject({ kind: "static", actor: "mcp" }); + }); + + it("the NEW LOOPOVER_API_TOKEN wins when BOTH the legacy and new names are set", async () => { + const env = createTestEnv({ LOOPOVER_API_TOKEN: "new-api-token" }); + // The winning (new) value authenticates. + await expect(authenticatePrivateToken(env, "new-api-token")).resolves.toMatchObject({ kind: "static", actor: "api" }); + // The shadowed legacy value must NOT authenticate once the new name is set. + await expect(authenticatePrivateToken(env, "test-api-token")).resolves.toBeNull(); + }); + it("scopes MCP static-token actuation to an explicit repo allowlist, denying by default (#2253)", () => { // Unset/empty ⇒ deny (fail closed — the shared GITTENSORY_MCP_TOKEN must not implicitly actuate everywhere). expect(isMcpActuationRepoAllowed(undefined, "owner/repo")).toBe(false); diff --git a/test/unit/notify-discord.test.ts b/test/unit/notify-discord.test.ts index 6e525792c2..4afae8ec3f 100644 --- a/test/unit/notify-discord.test.ts +++ b/test/unit/notify-discord.test.ts @@ -67,6 +67,42 @@ describe("notify-discord resolveWebhook (modular self-host fallback)", () => { expect(calls).toEqual([HOOK]); }); + // #4774 dual-read: LOOPOVER_DISCORD_WEBHOOK is a first-class alias of the legacy per-repo secret name + // GITTENSORY_DISCORD_WEBHOOK, new name winning when both are set. + describe("#4774 GITTENSORY_ -> LOOPOVER_ dual-read (per-repo legacy secret name)", () => { + const NEW_HOOK = "https://discord.com/api/webhooks/456/def"; + + it("resolves via the NEW LOOPOVER_DISCORD_WEBHOOK alone (legacy unset)", () => { + expect(resolveDiscordWebhook(withEnv({ LOOPOVER_DISCORD_WEBHOOK: NEW_HOOK }), "JSONbored/gittensory")).toEqual({ + status: "configured", + url: NEW_HOOK, + source: "legacy_repo_secret", + }); + }); + + it("still resolves via the legacy GITTENSORY_DISCORD_WEBHOOK alone — an untouched .env keeps working unchanged", () => { + expect(resolveDiscordWebhook(withEnv({ GITTENSORY_DISCORD_WEBHOOK: HOOK }), "JSONbored/gittensory")).toEqual({ + status: "configured", + url: HOOK, + source: "legacy_repo_secret", + }); + }); + + it("the NEW LOOPOVER_DISCORD_WEBHOOK wins when BOTH are set", async () => { + const calls = stubFetch(); + await notify(withEnv({ GITTENSORY_DISCORD_WEBHOOK: HOOK, LOOPOVER_DISCORD_WEBHOOK: NEW_HOOK }), "JSONbored/gittensory"); + expect(calls).toEqual([NEW_HOOK]); + }); + + it("does not add a LOOPOVER_ companion for a repo secret outside the GITTENSORY_ family (METAGRAPHED_/AWESOME_)", async () => { + const calls = stubFetch(); + await notify(withEnv({ METAGRAPHED_DISCORD_WEBHOOK: HOOK, LOOPOVER_DISCORD_WEBHOOK: NEW_HOOK }), "JSONbored/metagraphed"); + // METAGRAPHED_DISCORD_WEBHOOK has no LOOPOVER_ companion of its own; the unrelated LOOPOVER_DISCORD_WEBHOOK + // value must not leak into a different repo's channel. + expect(calls).toEqual([HOOK]); + }); + }); + it("uses process.env as a self-host fallback when the runtime Env object does not carry the webhook", async () => { process.env.DISCORD_WEBHOOK_URL = FALLBACK; expect(resolveDiscordWebhook(createTestEnv(), "acme/widgets")).toEqual({ status: "configured", url: FALLBACK, source: "global" }); diff --git a/test/unit/notify-pagerduty.test.ts b/test/unit/notify-pagerduty.test.ts index f5acae6cb2..9bd2705799 100644 --- a/test/unit/notify-pagerduty.test.ts +++ b/test/unit/notify-pagerduty.test.ts @@ -60,6 +60,25 @@ describe("isPagerDutyEnabled", () => { it("treats anything else (including unset) as disabled", () => { for (const value of [undefined, "", "0", "false", "nah"]) expect(isPagerDutyEnabled({ GITTENSORY_ENABLE_PAGERDUTY: value })).toBe(false); }); + + // #4774 dual-read: LOOPOVER_ENABLE_PAGERDUTY is a first-class alias of the legacy GITTENSORY_ENABLE_PAGERDUTY. + describe("#4774 GITTENSORY_ -> LOOPOVER_ dual-read", () => { + it("enables via the NEW LOOPOVER_ name alone (legacy name unset)", () => { + expect(isPagerDutyEnabled({ LOOPOVER_ENABLE_PAGERDUTY: "true" })).toBe(true); + }); + it("still enables via the legacy GITTENSORY_ name alone — an untouched .env keeps working unchanged", () => { + expect(isPagerDutyEnabled({ GITTENSORY_ENABLE_PAGERDUTY: "true" })).toBe(true); + }); + it("the NEW LOOPOVER_ name wins when BOTH are set", () => { + // legacy says on, new name says off -> effective result must be OFF (new wins). + expect(isPagerDutyEnabled({ GITTENSORY_ENABLE_PAGERDUTY: "true", LOOPOVER_ENABLE_PAGERDUTY: "false" })).toBe(false); + // legacy says off, new name says on -> effective result must be ON (new wins). + expect(isPagerDutyEnabled({ GITTENSORY_ENABLE_PAGERDUTY: "false", LOOPOVER_ENABLE_PAGERDUTY: "true" })).toBe(true); + }); + it("blank LOOPOVER_ENABLE_PAGERDUTY falls through to the legacy name, not to disabled", () => { + expect(isPagerDutyEnabled({ GITTENSORY_ENABLE_PAGERDUTY: "true", LOOPOVER_ENABLE_PAGERDUTY: " " })).toBe(true); + }); + }); }); describe("resolvePagerDutyRoutingKey", () => { diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index fb03f10658..b64660b457 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -1871,6 +1871,25 @@ describe("subscription CLI helpers + fail-safe", () => { expect(metrics).not.toContain("loopover_ai_requests_total"); }); + // #4774 dual-read: LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER is a first-class alias of the legacy + // GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER opt-in, new name winning when both are set. + it("credential isolation opt-in accepts the NEW LOOPOVER_ name alone (legacy unset)", async () => { + const ok: StubSpawn = async () => ({ stdout: JSON.stringify({ type: "result", result: "ok" }), code: 0 }); + await expect(createCodexAi({ LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, ok, noAuthCheck).run("gpt-5", { prompt: "x" })).resolves.toMatchObject({ response: "ok" }); + }); + + it("credential isolation opt-in: the NEW LOOPOVER_ name wins when BOTH are set — a non-\"1\" new value still fails closed", async () => { + const shouldNotSpawn: StubSpawn = async () => { + throw new Error("spawned"); + }; + await expect( + createCodexAi( + { GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1", LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER: "0" }, + shouldNotSpawn, + ).run("gpt-5", { prompt: "x" }), + ).rejects.toThrow(/codex_credential_isolation_required/); + }); + it("resolveCodexAuthPath: CODEX_HOME wins, else HOME/.codex, else ~/.codex", () => { expect(resolveCodexAuthPath({ CODEX_HOME: "/data/codex", HOME: "/home/node" })).toBe( "/data/codex/auth.json", diff --git a/test/unit/selfhost-env-reference-script.test.ts b/test/unit/selfhost-env-reference-script.test.ts index 1266903f73..22b96291c5 100644 --- a/test/unit/selfhost-env-reference-script.test.ts +++ b/test/unit/selfhost-env-reference-script.test.ts @@ -53,6 +53,9 @@ function fixtureRoot(): string { "const helperOnly = envString(env, 'SERVICE_HELPER_ONLY');", "const casted = (env as unknown as Record).CASTED_ONLY;", "const parsedInt = parsePositiveIntEnv('PARSED_INT_ONLY', { min: 1, fallback: 4 });", + "const dual = dualPrefixEnvString(env, 'DUAL_ONLY');", + "const dualFlag = dualPrefixEnvFlag(parentEnv, 'DUAL_FLAG_ONLY');", + "const dualStrict = dualPrefixEnvStrictFlag((env as unknown as Record), 'DUAL_STRICT_ONLY');", "", ].join("\n"), ); @@ -69,7 +72,13 @@ describe("gen-selfhost-env-reference (#2081)", () => { { name: "DEFAULTED_ENV", firstReference: "src/selfhost/a.ts" }, { name: "DESTRUCTURED", firstReference: "src/selfhost/a.ts" }, { name: "FIRST", firstReference: "src/selfhost/a.ts" }, + { name: "GITTENSORY_DUAL_FLAG_ONLY", firstReference: "src/services/notify-discord.ts" }, + { name: "GITTENSORY_DUAL_ONLY", firstReference: "src/services/notify-discord.ts" }, + { name: "GITTENSORY_DUAL_STRICT_ONLY", firstReference: "src/services/notify-discord.ts" }, { name: "HELPER_ONLY", firstReference: "src/selfhost/a.ts" }, + { name: "LOOPOVER_DUAL_FLAG_ONLY", firstReference: "src/services/notify-discord.ts" }, + { name: "LOOPOVER_DUAL_ONLY", firstReference: "src/services/notify-discord.ts" }, + { name: "LOOPOVER_DUAL_STRICT_ONLY", firstReference: "src/services/notify-discord.ts" }, { name: "NESTED_ONLY", firstReference: "src/selfhost/nested/b.ts" }, { name: "OBJECT_ALIASED", firstReference: "src/selfhost/a.ts" }, { name: "OBJECT_BRACKET", firstReference: "src/selfhost/a.ts" }, diff --git a/test/unit/selfhost-health.test.ts b/test/unit/selfhost-health.test.ts index 9f8a394feb..53e98304d7 100644 --- a/test/unit/selfhost-health.test.ts +++ b/test/unit/selfhost-health.test.ts @@ -70,6 +70,34 @@ describe("codexAuthReadinessProbe (#GITTENSORY-C)", () => { ).toBeNull(); }); + // #4774 dual-read: LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER is a first-class alias of the legacy + // GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER, new name winning when both are set (strict "1"-only, matching + // this flag's intentionally narrow opt-in convention). + describe("#4774 GITTENSORY_ -> LOOPOVER_ dual-read", () => { + it("registers a probe via the NEW LOOPOVER_ name alone (legacy unset)", () => { + expect(codexAuthReadinessProbe({ LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, async () => ({ code: 0 }))).not.toBeNull(); + }); + it("still registers a probe via the legacy GITTENSORY_ name alone — an untouched .env keeps working unchanged", () => { + expect(codexAuthReadinessProbe({ GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, async () => ({ code: 0 }))).not.toBeNull(); + }); + it("the NEW LOOPOVER_ name wins when BOTH are set", () => { + // legacy says on ("1"), new name says off -> no probe (new wins). + expect( + codexAuthReadinessProbe( + { GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1", LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER: "0" }, + async () => ({ code: 0 }), + ), + ).toBeNull(); + // legacy says off, new name says on ("1") -> a probe registers (new wins). + expect( + codexAuthReadinessProbe( + { GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "0", LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, + async () => ({ code: 0 }), + ), + ).not.toBeNull(); + }); + }); + it("reports healthy only when BOTH codex --version exits 0 AND the auth file check passes", async () => { const probe = codexAuthReadinessProbe( { GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, diff --git a/test/unit/selfhost-otel.test.ts b/test/unit/selfhost-otel.test.ts index a187d044ca..977748645b 100644 --- a/test/unit/selfhost-otel.test.ts +++ b/test/unit/selfhost-otel.test.ts @@ -141,6 +141,31 @@ describe("self-host OpenTelemetry", () => { expect(otelMocks.OTLPTraceExporter).toHaveBeenCalledTimes(1); }); + // #4774 dual-read: LOOPOVER_VERSION is a first-class alias of the legacy GITTENSORY_VERSION for the + // `service.version` resource attribute, new name winning when both are set. + it("resolves service.version via the NEW LOOPOVER_VERSION alone (legacy unset)", async () => { + await initOpenTelemetry( + env({ OTEL_TRACES_EXPORTER: "otlp", OTEL_EXPORTER_OTLP_ENDPOINT: "http://otel-collector:4318", LOOPOVER_VERSION: "loopover-selfhost@test" }), + ); + await withOtelSpan("selfhost.queue.job", {}, async () => "ok"); + await flushOpenTelemetry(); + expect(otelMocks.exportedSpans[0].resource.attributes).toMatchObject({ "service.version": "loopover-selfhost@test" }); + }); + + it("the NEW LOOPOVER_VERSION wins over the legacy GITTENSORY_VERSION when BOTH are set", async () => { + await initOpenTelemetry( + env({ + OTEL_TRACES_EXPORTER: "otlp", + OTEL_EXPORTER_OTLP_ENDPOINT: "http://otel-collector:4318", + GITTENSORY_VERSION: "gittensory-selfhost@old", + LOOPOVER_VERSION: "loopover-selfhost@new", + }), + ); + await withOtelSpan("selfhost.queue.job", {}, async () => "ok"); + await flushOpenTelemetry(); + expect(otelMocks.exportedSpans[0].resource.attributes).toMatchObject({ "service.version": "loopover-selfhost@new" }); + }); + it("records failed spans and preserves nested parent context", async () => { await initOpenTelemetry(env({ OTEL_TRACES_EXPORTER: "otlp", diff --git a/test/unit/selfhost-preflight.test.ts b/test/unit/selfhost-preflight.test.ts index 4302d069fc..97042e2b75 100644 --- a/test/unit/selfhost-preflight.test.ts +++ b/test/unit/selfhost-preflight.test.ts @@ -298,6 +298,64 @@ describe("self-host environment preflight (#2080)", () => { ], }); }); + + // #4774 dual-read: GITTENSORY_API_TOKEN/GITTENSORY_MCP_TOKEN also accept a LOOPOVER_ companion name, new + // name winning when both are set. The preflight strength/placeholder/reuse checks below must judge the + // EFFECTIVE (dual-read-resolved) value, not just the legacy name — this is what keeps preflight in sync + // with the real auth gate in src/auth/security.ts. + describe("#4774 GITTENSORY_ -> LOOPOVER_ dual-read", () => { + it("accepts a strong secret supplied via the NEW LOOPOVER_ name alone (legacy name unset)", () => { + expect( + preflightEnv({ ...baseEnv, LOOPOVER_API_TOKEN: "api-token-value-with-plenty-of-entropy-2" }), + ).toEqual({ ok: true, problems: [] }); + }); + + it("still accepts a strong secret supplied via the legacy GITTENSORY_ name alone — an untouched .env keeps working unchanged", () => { + expect( + preflightEnv({ ...baseEnv, GITTENSORY_API_TOKEN: "api-token-value-with-plenty-of-entropy-2" }), + ).toEqual({ ok: true, problems: [] }); + }); + + it("judges the NEW LOOPOVER_ value when BOTH are set: a weak new value fails even though the legacy value is strong", () => { + const result = preflightEnv({ + ...baseEnv, + GITTENSORY_API_TOKEN: "api-token-value-with-plenty-of-entropy-2", + LOOPOVER_API_TOKEN: "too-short", + }); + expect(result).toEqual({ + ok: false, + problems: [expect.objectContaining({ var: "GITTENSORY_API_TOKEN", message: expect.stringContaining("too short") })], + }); + }); + + it("judges the NEW LOOPOVER_ value when BOTH are set: a strong new value passes even though the legacy value is a known placeholder", () => { + expect( + preflightEnv({ + ...baseEnv, + GITTENSORY_API_TOKEN: "change-this-32-byte-random-token", + LOOPOVER_API_TOKEN: "api-token-value-with-plenty-of-entropy-2", + }), + ).toEqual({ ok: true, problems: [] }); + }); + + it("still flags a reused value across LOOPOVER_API_TOKEN and GITTENSORY_MCP_TOKEN (dual-read resolves to the same underlying secret family)", () => { + const sharedSecret = "a-perfectly-strong-random-value-1234"; + const result = preflightEnv({ + ...baseEnv, + LOOPOVER_API_TOKEN: sharedSecret, + GITTENSORY_MCP_TOKEN: sharedSecret, + }); + expect(result).toEqual({ + ok: false, + problems: [ + expect.objectContaining({ + var: "GITTENSORY_MCP_TOKEN", + message: expect.stringContaining("must not reuse the same value as GITTENSORY_API_TOKEN"), + }), + ], + }); + }); + }); }); it("flags blank values and invalid DATABASE_URL while never echoing supplied secrets", () => { diff --git a/test/unit/selfhost-sentry.test.ts b/test/unit/selfhost-sentry.test.ts index b9828f1c9b..43195dcc66 100644 --- a/test/unit/selfhost-sentry.test.ts +++ b/test/unit/selfhost-sentry.test.ts @@ -522,6 +522,31 @@ describe("enabled when SENTRY_DSN is set", () => { ).toBe("custom@sha"); }); + // #4774 dual-read: LOOPOVER_VERSION is a first-class alias of the legacy GITTENSORY_VERSION for the + // image-baked release fallback, new name winning when both are set. + describe("#4774 GITTENSORY_ -> LOOPOVER_VERSION dual-read", () => { + it("falls back to the NEW LOOPOVER_VERSION alone (legacy unset)", () => { + expect( + resolveSentryRelease({ SENTRY_RELEASE: "", LOOPOVER_VERSION: "loopover-selfhost@0.2.0" } as unknown as NodeJS.ProcessEnv), + ).toBe("loopover-selfhost@0.2.0"); + }); + + it("still falls back to the legacy GITTENSORY_VERSION alone — an untouched .env keeps working unchanged", () => { + expect( + resolveSentryRelease({ SENTRY_RELEASE: "", GITTENSORY_VERSION: "gittensory-selfhost@0.1.0" } as unknown as NodeJS.ProcessEnv), + ).toBe("gittensory-selfhost@0.1.0"); + }); + + it("the NEW LOOPOVER_VERSION wins over the legacy GITTENSORY_VERSION when BOTH are set (SENTRY_RELEASE unset)", () => { + expect( + resolveSentryRelease({ + GITTENSORY_VERSION: "gittensory-selfhost@0.1.0", + LOOPOVER_VERSION: "loopover-selfhost@0.2.0", + } as unknown as NodeJS.ProcessEnv), + ).toBe("loopover-selfhost@0.2.0"); + }); + }); + it("captureError sends with context, tags operational fields, and without context skips setContext", async () => { await initSentry({ SENTRY_DSN: "d" } as unknown as NodeJS.ProcessEnv); captureError(new Error("boom"), { kind: "job_dead" }); diff --git a/test/unit/utils-env.test.ts b/test/unit/utils-env.test.ts new file mode 100644 index 0000000000..f7e5b1c4e7 --- /dev/null +++ b/test/unit/utils-env.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from "vitest"; +import { dualPrefixEnvFlag, dualPrefixEnvStrictFlag, dualPrefixEnvString } from "../../src/utils/env"; + +// #4774: GITTENSORY_ -> LOOPOVER_ self-host env var prefix rename, dual-read. This is a DUAL-READ addition, +// never a cutover -- an existing self-hoster's .env with only the legacy GITTENSORY_ name must keep working +// completely unchanged. The new LOOPOVER_ name wins when both are set (mirrors resolveSentryRelease's +// existing "explicit override first" precedent in src/selfhost/sentry.ts). +describe("dualPrefixEnvString", () => { + it("reads via the NEW LOOPOVER_ prefix alone (legacy unset)", () => { + expect(dualPrefixEnvString({ LOOPOVER_VERSION: "1.2.3" }, "VERSION")).toBe("1.2.3"); + }); + + it("still reads via the legacy GITTENSORY_ prefix alone — an untouched .env keeps working unchanged", () => { + expect(dualPrefixEnvString({ GITTENSORY_VERSION: "1.2.3" }, "VERSION")).toBe("1.2.3"); + }); + + it("the NEW LOOPOVER_ prefix wins when BOTH are set", () => { + expect( + dualPrefixEnvString({ GITTENSORY_VERSION: "old-value", LOOPOVER_VERSION: "new-value" }, "VERSION"), + ).toBe("new-value"); + }); + + it("returns undefined when neither prefix is set", () => { + expect(dualPrefixEnvString({}, "VERSION")).toBeUndefined(); + }); + + it("treats a blank/whitespace-only LOOPOVER_ value as unset and falls through to the legacy prefix", () => { + expect(dualPrefixEnvString({ GITTENSORY_VERSION: "old-value", LOOPOVER_VERSION: " " }, "VERSION")).toBe( + "old-value", + ); + }); + + it("trims surrounding whitespace off whichever value wins", () => { + expect(dualPrefixEnvString({ LOOPOVER_VERSION: " 1.2.3 " }, "VERSION")).toBe("1.2.3"); + expect(dualPrefixEnvString({ GITTENSORY_VERSION: " 1.2.3 " }, "VERSION")).toBe("1.2.3"); + }); + + it("treats a blank/whitespace-only legacy value the same as unset", () => { + expect(dualPrefixEnvString({ GITTENSORY_VERSION: " " }, "VERSION")).toBeUndefined(); + }); +}); + +describe("dualPrefixEnvFlag", () => { + it("accepts the codebase-standard truthy strings via either prefix, case-insensitively", () => { + for (const value of ["1", "true", "YES", "On"]) { + expect(dualPrefixEnvFlag({ GITTENSORY_ENABLE_PAGERDUTY: value }, "ENABLE_PAGERDUTY")).toBe(true); + expect(dualPrefixEnvFlag({ LOOPOVER_ENABLE_PAGERDUTY: value }, "ENABLE_PAGERDUTY")).toBe(true); + } + }); + + it("treats anything else (including unset) as disabled", () => { + for (const value of [undefined, "", "0", "false", "nah"]) { + expect(dualPrefixEnvFlag({ GITTENSORY_ENABLE_PAGERDUTY: value }, "ENABLE_PAGERDUTY")).toBe(false); + } + expect(dualPrefixEnvFlag({}, "ENABLE_PAGERDUTY")).toBe(false); + }); + + it("the NEW LOOPOVER_ prefix wins when BOTH are set", () => { + expect( + dualPrefixEnvFlag({ GITTENSORY_ENABLE_PAGERDUTY: "true", LOOPOVER_ENABLE_PAGERDUTY: "false" }, "ENABLE_PAGERDUTY"), + ).toBe(false); + expect( + dualPrefixEnvFlag({ GITTENSORY_ENABLE_PAGERDUTY: "false", LOOPOVER_ENABLE_PAGERDUTY: "true" }, "ENABLE_PAGERDUTY"), + ).toBe(true); + }); +}); + +describe("dualPrefixEnvStrictFlag", () => { + it("requires the exact string \"1\" — a loose-truthy value does NOT count", () => { + expect(dualPrefixEnvStrictFlag({ GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, "ENABLE_UNSAFE_CODEX_REVIEWER")).toBe(true); + expect(dualPrefixEnvStrictFlag({ GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "true" }, "ENABLE_UNSAFE_CODEX_REVIEWER")).toBe(false); + expect(dualPrefixEnvStrictFlag({}, "ENABLE_UNSAFE_CODEX_REVIEWER")).toBe(false); + }); + + it("still accepts the legacy GITTENSORY_ name alone — an untouched .env keeps working unchanged", () => { + expect(dualPrefixEnvStrictFlag({ GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, "ENABLE_UNSAFE_CODEX_REVIEWER")).toBe(true); + }); + + it("accepts the NEW LOOPOVER_ name alone", () => { + expect(dualPrefixEnvStrictFlag({ LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, "ENABLE_UNSAFE_CODEX_REVIEWER")).toBe(true); + }); + + it("the NEW LOOPOVER_ name wins when BOTH are set — does not silently broaden accepted values", () => { + // Legacy is the strict "1" (would enable on its own); new name is present but not "1" -> new wins -> disabled. + expect( + dualPrefixEnvStrictFlag( + { GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "1", LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER: "true" }, + "ENABLE_UNSAFE_CODEX_REVIEWER", + ), + ).toBe(false); + // New name is exactly "1" -> enabled, even though legacy differs. + expect( + dualPrefixEnvStrictFlag( + { GITTENSORY_ENABLE_UNSAFE_CODEX_REVIEWER: "0", LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER: "1" }, + "ENABLE_UNSAFE_CODEX_REVIEWER", + ), + ).toBe(true); + }); +});