From 84537c4dd976a89b6ec094f43ae3ef7bee12cb1a Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Mon, 2 Nov 2026 14:23:37 -0500 Subject: [PATCH] feat(server): read source control host tokens from a secret provider (#27756) Signed-off-by: Yordis Prieto Co-authored-by: maria-rcks --- .../src/provider/ProviderInstanceRegistry.ts | 20 +++-- .../server/src/secrets/settingsSecretValue.ts | 10 +-- apps/server/src/serverSettings.test.ts | 28 +++++-- .../sourceControlHostSecrets.test.ts | 23 +++++- .../sourceControl/sourceControlHostSecrets.ts | 11 ++- .../SourceControlAppSignInSettings.tsx | 4 +- .../settings/SourceControlTokenSettings.tsx | 48 ++++++++--- packages/contracts/src/settingsSecretValue.ts | 70 ++++++++++++---- .../src/shared/definition.ts | 3 +- .../src/server/GitHubCredentials.ts | 8 +- .../src/server/GitHubSourceControlProvider.ts | 7 +- .../src/server/gitHubDiscovery.test.ts | 82 ++++++++++++++++++- 12 files changed, 256 insertions(+), 58 deletions(-) diff --git a/apps/server/src/provider/ProviderInstanceRegistry.ts b/apps/server/src/provider/ProviderInstanceRegistry.ts index 57a218a0dafe1..cc3ca60fcb829 100644 --- a/apps/server/src/provider/ProviderInstanceRegistry.ts +++ b/apps/server/src/provider/ProviderInstanceRegistry.ts @@ -9,6 +9,8 @@ import { type SecretReference, secretReferencesInSettings, type ServerProvider, + type SettingsSecretSlot, + withSettingsSecrets, } from "@t3tools/contracts"; import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; @@ -45,7 +47,7 @@ export class ProviderInstanceRegistry extends Context.Service< interface InstanceSecrets { readonly environment: ResolvedProviderInstanceEnvironment; - readonly config: Readonly>; + readonly config: ReadonlyArray; } interface LiveEntry { @@ -110,12 +112,12 @@ const readInstanceSecrets = Effect.fn("ProviderInstanceRegistry.readInstanceSecr } return [{ ...variable, value: secret, sensitive: true }]; }); - const config: Record = {}; - for (const { field } of configFields) { + const config = configFields.flatMap(({ field, key }) => { const secret = values[index++]; - if (secret === undefined) unreadable.push(field); - else config[field] = secret; - } + if (secret !== undefined) return [{ field, key, value: secret }]; + unreadable.push(field); + return []; + }); const secrets: InstanceSecrets = { environment, config }; return { secrets, unreadable }; }); @@ -224,7 +226,7 @@ const buildEntry = Effect.fn("ProviderInstanceRegistry.buildEntry")(function* reference)); - const resolved = Object.fromEntries( - pending.map(({ field }, index) => [field, values[index] ?? ""] as const), + return withSettingsSecrets( + settings, + pending.map((slot, index) => ({ ...slot, value: values[index] ?? "" })), ); - return { ...settings, ...resolved }; }); diff --git a/apps/server/src/serverSettings.test.ts b/apps/server/src/serverSettings.test.ts index 281bab5e4227b..cc34e2f818381 100644 --- a/apps/server/src/serverSettings.test.ts +++ b/apps/server/src/serverSettings.test.ts @@ -1547,20 +1547,34 @@ it.layer(NodeServices.layer)("server settings", (it) => { provider: SecretProviderId.make("personal"), reference: "op://Private/Bitbucket/token", }; + const gitHubReference = { + provider: SecretProviderId.make("personal"), + reference: "op://Private/GitHub/token", + }; yield* serverSettings.updateSettings({ - sourceControlHosts: { bitbucket: { accessToken: "bb-access" } }, + sourceControlHosts: { + bitbucket: { accessToken: "bb-access" }, + github: { tokens: { "github.com": "ghp_dotcom", "ghe.acme.test": "ghp_ghe" } }, + }, }); assert.isTrue(Option.isSome(yield* stored("bitbucket", "accessToken", null))); + assert.isTrue(Option.isSome(yield* stored("github", "tokens", "github.com"))); const referenced = yield* serverSettings.updateSettings({ - sourceControlHosts: { bitbucket: { accessToken: reference } }, + sourceControlHosts: { + bitbucket: { accessToken: reference }, + github: { tokens: { "github.com": gitHubReference, "ghe.acme.test": "ghp_ghe" } }, + }, }); assert.deepEqual(referenced.sourceControlHosts.bitbucket?.accessToken, reference); assert.isTrue(Option.isNone(yield* stored("bitbucket", "accessToken", null))); - assert.deepEqual( - ServerSettingsModule.redactServerSettingsForClient(referenced).sourceControlHosts - .bitbucket?.accessToken, - reference, - ); + assert.isTrue(Option.isNone(yield* stored("github", "tokens", "github.com"))); + assert.isTrue(Option.isSome(yield* stored("github", "tokens", "ghe.acme.test"))); + const redacted = + ServerSettingsModule.redactServerSettingsForClient(referenced).sourceControlHosts; + assert.deepEqual(redacted.bitbucket?.accessToken, reference); + const tokens = redacted.github?.tokens as Record; + assert.deepEqual(tokens["github.com"], gitHubReference); + assert.notEqual(tokens["ghe.acme.test"], "ghp_ghe"); }).pipe(Effect.provide(layerServerSettingsWithSecrets())), ); diff --git a/apps/server/src/sourceControl/sourceControlHostSecrets.test.ts b/apps/server/src/sourceControl/sourceControlHostSecrets.test.ts index bfbe4a1c8ac94..be1da6f3d610c 100644 --- a/apps/server/src/sourceControl/sourceControlHostSecrets.test.ts +++ b/apps/server/src/sourceControl/sourceControlHostSecrets.test.ts @@ -14,7 +14,14 @@ const source = (reference: string) => ({ reference, }); -const secretsByReference = new Map([["bitbucket#token", "bb_from_provider"]]); +const secretsByReference = new Map([ + ["bitbucket#token", "bb_from_provider"], + ["github#token", "ghp_from_provider"], +]); + +const perHostHosts = { + github: { tokens: { "github.com": source("github#token"), "ghe.acme.test": "ghp_literal" } }, +}; const hosts = { bitbucket: { email: "me@example.test", accessToken: source("bitbucket#token") }, @@ -69,11 +76,25 @@ it.effect("leaves the resolver unused for a kind that holds no reference", () => }), ); +it.effect("replaces a per-host token reference and keeps the literal beside it", () => + Effect.gen(function* () { + const { reads, resolver } = countingResolver(); + const resolved = yield* resolveSourceControlHostSecrets(resolver, perHostHosts, "github"); + assert.deepEqual(resolved, { + github: { tokens: { "github.com": "ghp_from_provider", "ghe.acme.test": "ghp_literal" } }, + }); + assert.deepEqual(reads, ["github#token"]); + }), +); + it("collects the references held in credential fields and skips literal values", () => { assert.deepEqual(collectSourceControlHostSecretReferences(hosts), [ source("missing#token"), source("bitbucket#token"), ]); + assert.deepEqual(collectSourceControlHostSecretReferences(perHostHosts), [ + source("github#token"), + ]); assert.deepEqual( collectSourceControlHostSecretReferences({ bitbucket: { accessToken: "literal" }, diff --git a/apps/server/src/sourceControl/sourceControlHostSecrets.ts b/apps/server/src/sourceControl/sourceControlHostSecrets.ts index cf86ade6b0eee..82de25be4de69 100644 --- a/apps/server/src/sourceControl/sourceControlHostSecrets.ts +++ b/apps/server/src/sourceControl/sourceControlHostSecrets.ts @@ -53,7 +53,16 @@ export function writeSecretFieldValues( ): unknown { if (typeof original === "string") return values[0]?.[1] ?? ""; if (isSecretReference(original)) return original; - return Object.fromEntries(values.map(([serverHost, value]) => [serverHost ?? "", value])); + const sources = + original !== null && typeof original === "object" && !Array.isArray(original) + ? Object.entries(original).flatMap(([serverHost, entry]) => + isSecretReference(entry) ? [[serverHost.trim().toLowerCase(), entry] as const] : [], + ) + : []; + return Object.fromEntries([ + ...sources, + ...values.map(([serverHost, value]) => [serverHost ?? "", value] as const), + ]); } type SourceControlHostsSettings = Readonly>>>; diff --git a/apps/web/src/components/settings/SourceControlAppSignInSettings.tsx b/apps/web/src/components/settings/SourceControlAppSignInSettings.tsx index 5662954ef7cd6..df73fa3b24b45 100644 --- a/apps/web/src/components/settings/SourceControlAppSignInSettings.tsx +++ b/apps/web/src/components/settings/SourceControlAppSignInSettings.tsx @@ -101,7 +101,9 @@ export function SourceControlAppSignInSettings({ readHostCredentialSettings(settings, definition.kind), ); const signedIn = (signIns[host] ?? "").length > 0; - const tokenOverrides = (tokens[host] ?? "").length > 0; + const token = tokens[host]; + // A token read from a secret provider overrides the sign-in like a saved one. + const tokenOverrides = typeof token === "string" ? token.length > 0 : token !== undefined; const canSignIn = useAtomValue( sourceControlEnvironment.startSignIn.permissionAtom(environmentId), ); diff --git a/apps/web/src/components/settings/SourceControlTokenSettings.tsx b/apps/web/src/components/settings/SourceControlTokenSettings.tsx index 912de7ad91701..5a78b5f28287d 100644 --- a/apps/web/src/components/settings/SourceControlTokenSettings.tsx +++ b/apps/web/src/components/settings/SourceControlTokenSettings.tsx @@ -1,4 +1,9 @@ -import type { EnvironmentId } from "@t3tools/contracts"; +import { + isSecretReference, + type EnvironmentId, + type SecretReference, + type SettingsSecretValue, +} from "@t3tools/contracts"; import { ExternalLinkIcon } from "lucide-react"; import { useState } from "react"; @@ -12,8 +17,11 @@ import { serverEnvironment } from "../../state/server"; import { useAtomCommand } from "../../state/use-atom-command"; import { Button, InlineButton } from "../ui/button"; import { Input } from "../ui/input"; -import { SecretInput } from "./SecretInput"; import { Label } from "../ui/label"; +import { SecretSourceInput, useSecretSources } from "./SecretSourceInput"; + +const hasToken = (token: SettingsSecretValue | undefined) => + typeof token === "string" ? token.length > 0 : token !== undefined; export function SourceControlTokenSettings({ environmentId, @@ -34,15 +42,24 @@ export function SourceControlTokenSettings({ }); const defaultHost = definition.publicHost ?? ""; const [host, setHost] = useState(defaultHost); - const [draft, setDraft] = useState(""); + const secretSources = useSecretSources(environmentId); + const [pending, setPending] = useState<{ + readonly key: string; + readonly draft: string | SecretReference; + } | null>(null); const [saving, setSaving] = useState(false); const normalizedHost = host.trim().toLowerCase(); - const isSaved = (tokens[normalizedHost] ?? "").length > 0; + const savedToken = tokens[normalizedHost]; + const isSaved = hasToken(savedToken); const savedHosts = Object.entries(tokens) - .filter(([, value]) => value.length > 0) + .filter(([, value]) => hasToken(value)) .map(([saved]) => saved); + const sourceKey = JSON.stringify([normalizedHost, savedToken ?? ""]); + const draft = + pending?.key === sourceKey ? pending.draft : isSecretReference(savedToken) ? savedToken : ""; + const submission = typeof draft === "string" ? draft.trim() : draft; - const save = async (target: string, token: string) => { + const save = async (target: string, token: SettingsSecretValue) => { setSaving(true); const result = await updateSettings({ environmentId, @@ -55,7 +72,7 @@ export function SourceControlTokenSettings({ }); setSaving(false); if (result._tag === "Success") { - setDraft(""); + setPending(null); onSaved(); } }; @@ -65,7 +82,7 @@ export function SourceControlTokenSettings({ className="grid gap-3" onSubmit={(event) => { event.preventDefault(); - if (normalizedHost && draft.trim()) void save(normalizedHost, draft.trim()); + if (normalizedHost && submission !== "") void save(normalizedHost, submission); }} > {/* Locked while saving: a successful save clears the draft, which would drop edits made mid-request. */} @@ -106,12 +123,15 @@ export function SourceControlTokenSettings({
- setDraft(event.target.value)} + sources={secretSources} + onChange={(event) => setPending({ key: sourceKey, draft: event.target.value })} + onSecretChange={(secret) => setPending({ key: sourceKey, draft: secret ?? "" })} />
@@ -132,7 +152,11 @@ export function SourceControlTokenSettings({ Remove ) : null} - diff --git a/packages/contracts/src/settingsSecretValue.ts b/packages/contracts/src/settingsSecretValue.ts index 101c40998b6f0..97558c82d574f 100644 --- a/packages/contracts/src/settingsSecretValue.ts +++ b/packages/contracts/src/settingsSecretValue.ts @@ -26,26 +26,68 @@ export function acceptsSecretReference(schema: Schema.Top): boolean { return Schema.resolveAnnotationsKey(schema)?.settingsSecretReference === true; } -/** Fields of a settings struct typed `SettingsSecretValue`; any other schema has none. */ -function secretReferenceSettingsFields(schema: Schema.Top): ReadonlyArray { +const declaresSecretReference = (ast: SchemaAST.AST) => + ast.context?.annotations?.settingsSecretReference === true; + +interface SecretReferenceField { + readonly field: string; + readonly keyed: boolean; +} + +/** + * Fields of a settings struct typed `SettingsSecretValue`, or a record of it such as one value + * per host; any other schema has none. + */ +function secretReferenceSettingsFields(schema: Schema.Top): ReadonlyArray { if (!SchemaAST.isObjects(schema.ast)) return []; - return schema.ast.propertySignatures.flatMap((property) => - property.type.context?.annotations?.settingsSecretReference === true && - typeof property.name === "string" - ? [property.name] - : [], - ); + return schema.ast.propertySignatures.flatMap(({ name, type }) => { + if (typeof name !== "string") return []; + if (declaresSecretReference(type)) return [{ field: name, keyed: false }]; + return SchemaAST.isObjects(type) && + type.indexSignatures.some((signature) => declaresSecretReference(signature.type)) + ? [{ field: name, keyed: true }] + : []; + }); } -/** Each declared reference field of `settings` that holds a reference, in field order. */ +/** Where a reference sits: a field, or one key of a record field. */ +export interface SettingsSecretSlot { + readonly field: string; + readonly key: string | undefined; +} + +interface SettingsSecretReference extends SettingsSecretSlot { + readonly reference: SecretReference; +} + +/** Each declared reference slot of `settings` that holds a reference, in field order. */ export function secretReferencesInSettings( schema: Schema.Top, settings: unknown, -): ReadonlyArray<{ readonly field: string; readonly reference: SecretReference }> { +): ReadonlyArray { if (settings === null || typeof settings !== "object") return []; const values = settings as Readonly>; - return secretReferenceSettingsFields(schema).flatMap((field) => { - const value = values[field]; - return isSecretReference(value) ? [{ field, reference: value }] : []; - }); + return secretReferenceSettingsFields(schema).flatMap( + ({ field, keyed }) => { + const value = values[field]; + if (!keyed) + return isSecretReference(value) ? [{ field, key: undefined, reference: value }] : []; + if (value === null || typeof value !== "object") return []; + return Object.entries(value).flatMap(([key, entry]) => + isSecretReference(entry) ? [{ field, key, reference: entry }] : [], + ); + }, + ); +} + +/** `settings` with each slot set to its value, keeping the other keys of a record field. */ +export function withSettingsSecrets>>( + settings: Settings, + slots: ReadonlyArray, +): Settings { + const next: Record = { ...settings }; + for (const { field, key, value } of slots) { + next[field] = key === undefined ? value : { ...(next[field] as object), [key]: value }; + } + return next as Settings; } diff --git a/packages/source-control-core/src/shared/definition.ts b/packages/source-control-core/src/shared/definition.ts index 30e88377590f8..ed12267c25a09 100644 --- a/packages/source-control-core/src/shared/definition.ts +++ b/packages/source-control-core/src/shared/definition.ts @@ -3,6 +3,7 @@ import { type PullRequestReviewVerdict, type RepositoryIdentity, secretSetting, + SettingsSecretValue, SourceControlProviderKind, type SourceControlProviderInfo, TrimmedNonEmptyString, @@ -214,7 +215,7 @@ export const hostCredentialSettingsFields = { Schema.withDecodingDefault(Effect.succeed({})), Schema.annotateKey({ providerSettingsForm: { hidden: true } }), ), - tokens: Schema.Record(SourceControlHostName, TrimmedString).pipe( + tokens: Schema.Record(SourceControlHostName, SettingsSecretValue).pipe( Schema.withDecodingDefault(Effect.succeed({})), secretSetting, Schema.annotateKey({ providerSettingsForm: { hidden: true } }), diff --git a/packages/source-control-github/src/server/GitHubCredentials.ts b/packages/source-control-github/src/server/GitHubCredentials.ts index 41641790aac52..607e6ca00d1b1 100644 --- a/packages/source-control-github/src/server/GitHubCredentials.ts +++ b/packages/source-control-github/src/server/GitHubCredentials.ts @@ -16,7 +16,7 @@ import * as HostProcess from "@t3tools/shared/HostProcess"; import * as SourceControlHost from "@t3tools/source-control-core/server/SourceControlHost"; import { readSourceControlHostSettings } from "@t3tools/source-control-core/shared/definition"; -import type { ServerSettings } from "@t3tools/contracts"; +import { settingsSecretLiteral, type ServerSettings } from "@t3tools/contracts"; import * as GitHubClient from "../shared/definition.ts"; @@ -194,8 +194,10 @@ export const make = Effect.gen(function* () { ); const savedToken = (host: string) => - sourceControlHost.settings.get.pipe( - Effect.map((settings) => readSettings(settings).tokens[host]?.trim() || null), + sourceControlHost.settings.getWithSecrets(GitHubClient.definition.kind).pipe( + Effect.map( + (settings) => settingsSecretLiteral(readSettings(settings).tokens[host]).trim() || null, + ), Effect.orElseSucceed(() => null), ); diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts index bc0438cf41b60..8583797a9688b 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts @@ -1,4 +1,4 @@ -import { SourceControlProviderKind } from "@t3tools/contracts"; +import { settingsSecretLiteral, SourceControlProviderKind } from "@t3tools/contracts"; import * as Data from "effect/Data"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; @@ -225,7 +225,7 @@ export const makeDiscovery = Effect.gen(function* () { probe: Effect.fn("GitHubSourceControlProvider.discovery")(function* (cwd: string) { const settings = readSourceControlHostSettings( GitHubClient.settings, - yield* sourceControlHost.settings.get.pipe( + yield* sourceControlHost.settings.getWithSecrets(GitHubClient.definition.kind).pipe( Effect.map((current) => current.sourceControlHosts[GitHubClient.definition.kind]), Effect.orElseSucceed(() => undefined), ), @@ -237,7 +237,8 @@ export const makeDiscovery = Effect.gen(function* () { }, spec: { ...discovery, parseAuth: (input) => parseGitHubAuth(input, settings) }, }); - const savedToken = (settings.tokens["github.com"] ?? "").trim() !== ""; + const token = settings.tokens["github.com"]; + const savedToken = settingsSecretLiteral(token).trim() !== ""; const signedIn = appSignIn.enabled && !savedToken && (settings.signIns["github.com"] ?? "").trim() !== ""; const variable = environmentTokenVariable(environment); diff --git a/packages/source-control-github/src/server/gitHubDiscovery.test.ts b/packages/source-control-github/src/server/gitHubDiscovery.test.ts index cd8a838b23723..b108d71f75584 100644 --- a/packages/source-control-github/src/server/gitHubDiscovery.test.ts +++ b/packages/source-control-github/src/server/gitHubDiscovery.test.ts @@ -1,10 +1,14 @@ -import { SourceControlProviderKind } from "@t3tools/contracts"; +import { ServerSettings, SourceControlProviderKind } from "@t3tools/contracts"; import * as HostProcess from "@t3tools/shared/HostProcess"; import { assert, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; import * as Redacted from "effect/Redacted"; +import * as Schema from "effect/Schema"; +import * as ChildProcessSpawner from "effect/process/ChildProcessSpawner"; import * as TestSourceControlHost from "@t3tools/source-control-testing/TestSourceControlHost"; +import * as SourceControlHost from "@t3tools/source-control-core/server/SourceControlHost"; import * as GitHubCredentials from "./GitHubCredentials.ts"; import * as GitHubAppSignIn from "./GitHubAppSignIn.ts"; @@ -79,3 +83,79 @@ it.effect("asks for no credential when the identity resolver left the base URL e assert.deepStrictEqual(hosts, []); }), ); + +const decodeSettings = Schema.decodeSync(ServerSettings); +const referencedTokenSettings = decodeSettings({ + sourceControlHosts: { + github: { tokens: { "github.com": { provider: "vault", reference: "op://v/gh" } } }, + }, +}); + +it.effect.each([ + { name: "readable", token: "resolved-token", source: "the token saved in Settings" }, + { name: "unreadable", token: "", source: "GH_TOKEN from the server environment" }, +])("checks the $name GitHub token reference against the API", (scenario) => + Effect.gen(function* () { + const paths: Array = []; + const discovery = yield* GitHubSourceControlProvider.makeDiscovery.pipe( + Effect.provide( + Layer.mergeAll( + Layer.mock(GitHubRest.GitHubRest)({ + rest: (input) => { + paths.push(input.path); + return Effect.succeed({ + status: 200, + headers: {}, + body: JSON.stringify({ login: "octocat" }), + truncated: false, + invalidUtf8: false, + }); + }, + }), + Layer.mock(GitHubAppSignIn.GitHubAppSignIn)({ enabled: false }), + Layer.effect( + SourceControlHost.SourceControlHost, + Effect.gen(function* () { + const host = yield* SourceControlHost.SourceControlHost; + return { + ...host, + settings: { + ...host.settings, + getWithSecrets: () => + Effect.succeed( + decodeSettings({ + sourceControlHosts: { + github: { tokens: { "github.com": scenario.token } }, + }, + }), + ), + }, + }; + }), + ).pipe( + Layer.provide( + TestSourceControlHost.layer({ + settings: referencedTokenSettings, + process: { + run: () => + Effect.succeed( + TestSourceControlHost.processOutput("", { + exitCode: ChildProcessSpawner.ExitCode(1), + }), + ), + }, + }), + ), + ), + ), + ), + Effect.provideService(HostProcess.Environment, { GH_TOKEN: "environment-token" }), + ); + if (discovery.type !== "managed-cli") return assert.fail("expected a managed CLI"); + const item = yield* discovery.probe("/repo"); + assert.deepStrictEqual(paths, ["user"]); + assert.strictEqual(item.auth.status, "authenticated"); + assert.deepStrictEqual(item.auth.account, Option.some("octocat")); + assert.include(Option.getOrThrow(item.auth.detail), scenario.source); + }), +);