From b4f8160ef6a904f82a977df68bfed66dc6869fb9 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Mon, 23 Mar 2026 22:34:46 +0100 Subject: [PATCH 1/4] fix: remove plaintext token storage and fix upgrade losing login - Stop writing tokens to ~/.archgate/credentials metadata file - Query git credential manager first in loadCredentials(), independent of the metadata file, so credentials survive upgrades - Add verification round-trip after git credential approve to detect misconfigured credential helpers (approve exits 0 even with no helper) - Auto-migrate legacy plaintext tokens to git credential manager on load - Make GIT_CREDENTIAL_ENV a function to pick up test-time env overrides - Isolate credential tests from system credential store on Windows via GIT_CONFIG_NOSYSTEM and GIT_CONFIG_GLOBAL --- .../agent-memory/archgate-developer/MEMORY.md | 2 + src/helpers/credential-store.ts | 268 ++++++++++-------- tests/helpers/auth.test.ts | 34 ++- tests/helpers/credential-store.test.ts | 74 ++++- 4 files changed, 238 insertions(+), 140 deletions(-) diff --git a/.claude/agent-memory/archgate-developer/MEMORY.md b/.claude/agent-memory/archgate-developer/MEMORY.md index 12770e5c..35a7105a 100644 --- a/.claude/agent-memory/archgate-developer/MEMORY.md +++ b/.claude/agent-memory/archgate-developer/MEMORY.md @@ -38,6 +38,8 @@ Skipping steps 2 or 3 is a workflow violation. The user should NEVER have to inv - **npm `main` field always gets included in publish** — `"main"` in `package.json` is always included in `npm publish` regardless of the `files` array. If the package doesn't need a default entry point (only sub-path exports like `./rules`), remove `main` entirely to avoid bundling the CLI entry point into the npm package. - **`CHANGELOG.md` is auto-generated — exclude from Prettier** — `CHANGELOG.md` is written by `TrigenSoftware/simple-release-action` during the release PR workflow. It is committed directly and never formatted by Prettier. It MUST be in `.prettierignore`; otherwise `bun run format:check` (part of `bun run validate`) will fail on the release PR. Do not attempt to run prettier on it post-commit. - **`mock.module("node:fetch", ...)` does NOT intercept `globalThis.fetch` in Bun** — Bun's runtime fetch is `globalThis.fetch`, not the `node:fetch` module. Using `mock.module` silently fails; the real network is hit. Always mock fetch by assigning `globalThis.fetch = mockFn as unknown as typeof fetch` directly, and restore via `mock.restore()` in `afterEach`. TypeScript type cast: `as never` is insufficient for the `typeof fetch` type (which includes `preconnect`) — always use `as unknown as typeof fetch`. Also captured in ARCH-005 Don'ts. +- **Git credential tests need system-level isolation on Windows** — Overriding `Bun.env.HOME` is NOT sufficient to isolate `git credential fill/approve` calls in tests. Windows Credential Manager is a system-level API, not file-based. Tests MUST set `Bun.env.GIT_CONFIG_NOSYSTEM = "1"` and `Bun.env.GIT_CONFIG_GLOBAL = ` to prevent git from reading the real credential helper config. Without this, tests on machines with stored credentials will pick up real tokens. +- **Module-level `{ ...Bun.env }` captures env at import time** — Spreading `Bun.env` into a module-level constant freezes the env snapshot. Tests that override `Bun.env.HOME` after import won't affect the constant. Fix: use a function that returns `{ ...Bun.env, ... }` on each call so it picks up test-time overrides. Applied in `src/helpers/credential-store.ts`. ## Validation Pipeline diff --git a/src/helpers/credential-store.ts b/src/helpers/credential-store.ts index abcbf637..07154aef 100644 --- a/src/helpers/credential-store.ts +++ b/src/helpers/credential-store.ts @@ -1,18 +1,11 @@ /** * credential-store.ts — Secure credential storage using git's native credential helpers. * - * Stores archgate tokens in the user's configured git credential manager - * (macOS Keychain, Windows Credential Manager, libsecret, etc.) using the - * standard `git credential approve/fill/reject` protocol. - * - * This means: - * - Tokens are encrypted at rest by the OS - * - `git clone https://plugins.archgate.dev/archgate.git` works transparently - * (git retrieves the stored credentials automatically) - * - No custom credential helper command needed — git already knows how to do this - * - * A lightweight JSON file at ~/.archgate/credentials stores non-sensitive - * metadata (github_user, created_at) for `archgate login status` display. + * Tokens are stored in the OS credential manager (macOS Keychain, Windows + * Credential Manager, libsecret) via `git credential approve/fill/reject`. + * A lightweight JSON metadata file at ~/.archgate/credentials stores only + * non-sensitive data (github_user, created_at). Tokens are NEVER written + * to disk in plaintext. * * @see https://git-scm.com/docs/git-credential */ @@ -24,17 +17,12 @@ import { internalPath, createPathIfNotExists } from "./paths"; const CREDENTIAL_HOST = "plugins.archgate.dev"; const METADATA_FILE = "credentials"; +const CREDENTIAL_TIMEOUT_MS = 3_000; -/** - * Environment variables for git credential commands. - * - GIT_TERMINAL_PROMPT=0 → suppress terminal prompts - * - GCM_INTERACTIVE=never → suppress GUI prompts (Git Credential Manager on Windows) - */ -const GIT_CREDENTIAL_ENV = { - ...Bun.env, - GIT_TERMINAL_PROMPT: "0", - GCM_INTERACTIVE: "never", -}; +/** Build env for git credential commands at call time (not import time). */ +function gitCredentialEnv(): Record { + return { ...Bun.env, GIT_TERMINAL_PROMPT: "0", GCM_INTERACTIVE: "never" }; +} export interface StoredCredentials { token: string; @@ -42,60 +30,49 @@ export interface StoredCredentials { created_at: string; } +/** Metadata file shape. Legacy files may have a `token` field (auto-migrated). */ +interface CredentialMetadata { + github_user: string; + created_at: string; + /** @deprecated Auto-migrated to git credential manager on load. */ + token?: string; +} + // --------------------------------------------------------------------------- // Git credential protocol helpers // --------------------------------------------------------------------------- -/** - * Store credentials in the user's git credential manager. - * Uses `git credential approve` which writes to the configured credential.helper. - */ +function credentialInput(username?: string, password?: string): string { + const lines = ["protocol=https", `host=${CREDENTIAL_HOST}`]; + if (username) lines.push(`username=${username}`); + if (password) lines.push(`password=${password}`); + lines.push("", ""); + return lines.join("\n"); +} + async function gitCredentialApprove( username: string, password: string ): Promise { - const input = [ - "protocol=https", - `host=${CREDENTIAL_HOST}`, - `username=${username}`, - `password=${password}`, - "", - "", - ].join("\n"); - const proc = Bun.spawn(["git", "credential", "approve"], { - stdin: new Blob([input]), + stdin: new Blob([credentialInput(username, password)]), stdout: "pipe", stderr: "pipe", - env: GIT_CREDENTIAL_ENV, + env: gitCredentialEnv(), }); return (await proc.exited) === 0; } -/** Timeout for git credential operations (3 seconds). */ -const CREDENTIAL_TIMEOUT_MS = 3_000; - -/** - * Retrieve credentials from the user's git credential manager. - * Uses `git credential fill` which reads from the configured credential.helper. - * - * GIT_TERMINAL_PROMPT=0 prevents git from prompting interactively. - * A timeout guard prevents hangs when the credential manager is unresponsive. - */ async function gitCredentialFill(): Promise<{ username: string; password: string; } | null> { - const input = ["protocol=https", `host=${CREDENTIAL_HOST}`, "", ""].join( - "\n" - ); - try { const proc = Bun.spawn(["git", "credential", "fill"], { - stdin: new Blob([input]), + stdin: new Blob([credentialInput()]), stdout: "pipe", stderr: "pipe", - env: GIT_CREDENTIAL_ENV, + env: gitCredentialEnv(), }); const result = await Promise.race([ @@ -118,147 +95,190 @@ async function gitCredentialFill(): Promise<{ if (line.startsWith("username=")) username = line.slice(9); if (line.startsWith("password=")) password = line.slice(9); } - return username && password ? { username, password } : null; } catch { return null; } } -/** - * Remove credentials from the user's git credential manager. - * Uses `git credential reject` which tells the configured helper to erase them. - */ async function gitCredentialReject( username: string, password: string ): Promise { - const input = [ - "protocol=https", - `host=${CREDENTIAL_HOST}`, - `username=${username}`, - `password=${password}`, - "", - "", - ].join("\n"); - const proc = Bun.spawn(["git", "credential", "reject"], { - stdin: new Blob([input]), + stdin: new Blob([credentialInput(username, password)]), stdout: "pipe", stderr: "pipe", - env: GIT_CREDENTIAL_ENV, + env: gitCredentialEnv(), }); await proc.exited; } // --------------------------------------------------------------------------- -// Metadata file (non-sensitive: github_user, created_at) +// Metadata helpers // --------------------------------------------------------------------------- function metadataPath(): string { return internalPath(METADATA_FILE); } +async function readMetadata(): Promise { + const file = Bun.file(metadataPath()); + if (!(await file.exists())) return null; + try { + const data = (await file.json()) as CredentialMetadata; + return data.github_user ? data : null; + } catch { + logDebug("Failed to parse credentials file"); + return null; + } +} + +async function writeMetadata(metadata: CredentialMetadata): Promise { + createPathIfNotExists(internalPath()); + const filePath = metadataPath(); + const clean: CredentialMetadata = { + github_user: metadata.github_user, + created_at: metadata.created_at, + }; + await Bun.write(filePath, JSON.stringify(clean, null, 2) + "\n"); + try { + chmodSync(filePath, 0o600); + } catch { + // chmod may fail on Windows — NTFS uses ACLs instead + } +} + // --------------------------------------------------------------------------- // Public API // --------------------------------------------------------------------------- +const CREDENTIAL_HELPER_HINT = + "Run `git config --global credential.helper` to check your configuration."; + /** * Persist archgate credentials securely. * - * - **Token** → git credential manager (encrypted at rest by the OS) - * - **Metadata** (github_user, created_at) → `~/.archgate/credentials` - * - * After this, `git clone https://plugins.archgate.dev/archgate.git` will - * automatically use the stored token — no credentials in the URL needed. + * Token goes to the git credential manager; only non-sensitive metadata is + * written to disk. A verification round-trip confirms the token was actually + * persisted — `git credential approve` exits 0 even without a configured + * helper, silently storing nothing. */ export async function saveCredentials( credentials: StoredCredentials ): Promise { - // Store token in git credential manager const stored = await gitCredentialApprove( credentials.github_user, credentials.token ); + if (stored) { - logDebug("Token stored in git credential manager"); + const verified = await gitCredentialFill(); + if (verified) { + logDebug("Token verified in git credential manager"); + } else { + logWarn( + "Token could not be verified in git credential manager.", + "Your credential helper may not persist credentials.", + CREDENTIAL_HELPER_HINT, + "Without a working credential helper, you will need to re-login after each session." + ); + } } else { - logDebug("git credential approve failed — token may not be persisted"); + logWarn( + "git credential approve failed.", + "Your git credential helper may not be configured.", + CREDENTIAL_HELPER_HINT + ); } - // Store metadata in ~/.archgate/credentials (for `login status` display). - // DEPRECATED: The token is also written to this file as a fallback for systems - // without a git credential manager. In the next major version, only metadata - // (github_user, created_at) will be written — the token field will be removed. - createPathIfNotExists(internalPath()); - const filePath = metadataPath(); - await Bun.write(filePath, JSON.stringify(credentials, null, 2) + "\n"); - try { - chmodSync(filePath, 0o600); - } catch { - // chmod may fail on Windows — NTFS uses ACLs instead - } - logDebug("Credentials metadata saved to", filePath); + await writeMetadata({ + github_user: credentials.github_user, + created_at: credentials.created_at, + }); + logDebug("Credentials metadata saved to", metadataPath()); } /** * Load stored archgate credentials, or null if none exist. * - * Reads the token from git's credential manager first, falling back to - * the plaintext file for legacy installs. + * Token is always read from the git credential manager. If a legacy metadata + * file contains a plaintext token, it is auto-migrated to the credential + * manager and scrubbed from disk. */ export async function loadCredentials(): Promise { - const file = Bun.file(metadataPath()); - if (!(await file.exists())) { - return null; - } - - let data: StoredCredentials; - try { - data = (await file.json()) as StoredCredentials; - if (!data.github_user) return null; - } catch { - logDebug("Failed to parse credentials file"); - return null; - } - - // Try to load token from git credential manager + // Git credential manager is the authoritative token source — check it first + // so credentials survive even if the metadata file is gone (e.g. after upgrade). const gitCreds = await gitCredentialFill(); + const metadata = await readMetadata(); + if (gitCreds) { + // Scrub legacy plaintext token from metadata file if present. + if (metadata?.token) await writeMetadata(metadata); + return { token: gitCreds.password, github_user: gitCreds.username, - created_at: data.created_at, + created_at: metadata?.created_at ?? "", }; } - // DEPRECATED: Fall back to token in the plaintext file (legacy storage). - // This fallback will be removed in the next major version. Users on systems - // without a git credential manager should run `archgate login refresh` to - // migrate their token to the credential manager. - if (!data.token) return null; - logWarn( - "Token loaded from plaintext file (deprecated).", - "Run `archgate login refresh` to migrate to secure credential storage." - ); - return data; + // No git creds — attempt to migrate a legacy plaintext token. + if (metadata?.token) { + logWarn("Migrating plaintext token to git credential manager..."); + const migrated = await gitCredentialApprove( + metadata.github_user, + metadata.token + ); + if (migrated) { + const verified = await gitCredentialFill(); + if (verified) { + logDebug("Legacy token migrated to git credential manager"); + const { token } = metadata; + await writeMetadata(metadata); + return { + token, + github_user: metadata.github_user, + created_at: metadata.created_at, + }; + } + } + logWarn( + "Could not migrate token to git credential manager.", + "Your credential helper may not be configured.", + "Run `archgate login refresh` to re-authenticate." + ); + return null; + } + + return null; } /** * Remove stored credentials (logout). - * - * Clears both the git credential manager and the metadata file. + * Clears both the git credential manager and the metadata file, + * including any legacy plaintext tokens. */ export async function clearCredentials(): Promise { - // Remove from git credential manager (need current credentials to reject) const gitCreds = await gitCredentialFill(); if (gitCreds) { await gitCredentialReject(gitCreds.username, gitCreds.password); logDebug("Token removed from git credential manager"); } - // Remove metadata file - if (await Bun.file(metadataPath()).exists()) { + const file = Bun.file(metadataPath()); + if (await file.exists()) { + try { + const metadata = (await file.json()) as CredentialMetadata; + if (metadata.token && metadata.github_user) { + await gitCredentialReject(metadata.github_user, metadata.token); + logDebug( + "Legacy plaintext token also rejected from credential manager" + ); + } + } catch { + // Metadata file is invalid — just delete it + } unlinkSync(metadataPath()); logDebug("Credentials file removed"); } diff --git a/tests/helpers/auth.test.ts b/tests/helpers/auth.test.ts index bf35c3a7..615fafd2 100644 --- a/tests/helpers/auth.test.ts +++ b/tests/helpers/auth.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test, beforeEach, afterEach, mock } from "bun:test"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -12,23 +12,34 @@ describe("auth", () => { let tempDir: string; let originalHome: string | undefined; let originalUserProfile: string | undefined; + let originalGitConfigNoSystem: string | undefined; + let originalGitConfigGlobal: string | undefined; beforeEach(() => { tempDir = mkdtempSync(join(tmpdir(), "archgate-auth-test-")); originalHome = Bun.env.HOME; originalUserProfile = Bun.env.USERPROFILE; + originalGitConfigNoSystem = Bun.env.GIT_CONFIG_NOSYSTEM; + originalGitConfigGlobal = Bun.env.GIT_CONFIG_GLOBAL; Bun.env.HOME = tempDir; Bun.env.USERPROFILE = tempDir; + // Isolate git credential operations from the system credential store. + Bun.env.GIT_CONFIG_NOSYSTEM = "1"; + const emptyGitConfig = join(tempDir, ".gitconfig"); + writeFileSync(emptyGitConfig, ""); + Bun.env.GIT_CONFIG_GLOBAL = emptyGitConfig; }); afterEach(() => { Bun.env.HOME = originalHome; Bun.env.USERPROFILE = originalUserProfile; + Bun.env.GIT_CONFIG_NOSYSTEM = originalGitConfigNoSystem; + Bun.env.GIT_CONFIG_GLOBAL = originalGitConfigGlobal; rmSync(tempDir, { recursive: true, force: true }); }); describe("saveCredentials / loadCredentials", () => { - test("round-trips credentials to ~/.archgate/credentials", async () => { + test("saves metadata without token and round-trips via git credential manager", async () => { const { saveCredentials, loadCredentials } = await import("../../src/helpers/credential-store"); @@ -38,14 +49,19 @@ describe("auth", () => { created_at: "2026-01-15", }); + // Metadata file must not contain the token + const credPath = join(tempDir, ".archgate", "credentials"); + const metadata = await Bun.file(credPath).json(); + expect(metadata.token).toBeUndefined(); + expect(metadata.github_user).toBe("testuser"); + + // With isolated git config (no credential helper), loadCredentials + // returns null because the token cannot be retrieved from the OS. const loaded = await loadCredentials(); - expect(loaded).not.toBeNull(); - expect(loaded!.token).toBe("ag_beta_abc123"); - expect(loaded!.github_user).toBe("testuser"); - expect(loaded!.created_at).toBe("2026-01-15"); + expect(loaded).toBeNull(); }); - test("returns null when no credentials file exists", async () => { + test("returns null when no credentials exist anywhere", async () => { const { loadCredentials } = await import("../../src/helpers/credential-store"); @@ -92,6 +108,10 @@ describe("auth", () => { }); await clearCredentials(); + + const credPath = join(tempDir, ".archgate", "credentials"); + expect(await Bun.file(credPath).exists()).toBe(false); + const loaded = await loadCredentials(); expect(loaded).toBeNull(); }); diff --git a/tests/helpers/credential-store.test.ts b/tests/helpers/credential-store.test.ts index 4b46c9ba..168bf1b1 100644 --- a/tests/helpers/credential-store.test.ts +++ b/tests/helpers/credential-store.test.ts @@ -1,25 +1,36 @@ import { describe, expect, test, beforeEach, afterEach } from "bun:test"; -import { mkdtempSync, mkdirSync, rmSync } from "node:fs"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; describe("credential-store", () => { let tempDir: string; let originalHome: string | undefined; + let originalGitConfigNoSystem: string | undefined; + let originalGitConfigGlobal: string | undefined; beforeEach(() => { tempDir = mkdtempSync(join(tmpdir(), "archgate-credstore-test-")); originalHome = Bun.env.HOME; + originalGitConfigNoSystem = Bun.env.GIT_CONFIG_NOSYSTEM; + originalGitConfigGlobal = Bun.env.GIT_CONFIG_GLOBAL; Bun.env.HOME = tempDir; + // Isolate git credential operations from the system credential store. + Bun.env.GIT_CONFIG_NOSYSTEM = "1"; + const emptyGitConfig = join(tempDir, ".gitconfig"); + writeFileSync(emptyGitConfig, ""); + Bun.env.GIT_CONFIG_GLOBAL = emptyGitConfig; }); afterEach(() => { Bun.env.HOME = originalHome; + Bun.env.GIT_CONFIG_NOSYSTEM = originalGitConfigNoSystem; + Bun.env.GIT_CONFIG_GLOBAL = originalGitConfigGlobal; rmSync(tempDir, { recursive: true, force: true }); }); describe("saveCredentials", () => { - test("writes metadata file to ~/.archgate/credentials", async () => { + test("writes metadata file WITHOUT token to ~/.archgate/credentials", async () => { const { saveCredentials } = await import("../../src/helpers/credential-store"); @@ -36,11 +47,13 @@ describe("credential-store", () => { const data = await file.json(); expect(data.github_user).toBe("testuser"); expect(data.created_at).toBe("2026-01-15"); + // Token must NOT be present in the metadata file + expect(data.token).toBeUndefined(); }); }); describe("loadCredentials", () => { - test("returns null when no credentials file exists", async () => { + test("returns null when no credentials file and no git creds exist", async () => { const { loadCredentials } = await import("../../src/helpers/credential-store"); @@ -66,14 +79,14 @@ describe("credential-store", () => { mkdirSync(join(tempDir, ".archgate"), { recursive: true }); await Bun.write( join(tempDir, ".archgate", "credentials"), - JSON.stringify({ token: "abc", created_at: "2026-01-01" }) + JSON.stringify({ created_at: "2026-01-01" }) ); const result = await loadCredentials(); expect(result).toBeNull(); }); - test("falls back to token in file when git credential fill fails", async () => { + test("attempts migration when metadata file has legacy plaintext token", async () => { const { loadCredentials } = await import("../../src/helpers/credential-store"); @@ -87,11 +100,34 @@ describe("credential-store", () => { }) ); + // On test systems without a git credential helper, migration will fail + // and loadCredentials returns null (no insecure plaintext fallback) const result = await loadCredentials(); - expect(result).not.toBeNull(); - expect(result!.github_user).toBe("testuser"); - // Token comes from either git credential manager or file fallback - expect(result!.token).toBeTruthy(); + // Result depends on whether git credential helper is configured. + // On CI/test systems it's typically null (migration fails gracefully). + if (result) { + expect(result.github_user).toBe("testuser"); + expect(result.token).toBeTruthy(); + } + }); + + test("does not return credentials from plaintext file when git creds fail", async () => { + const { loadCredentials } = + await import("../../src/helpers/credential-store"); + + mkdirSync(join(tempDir, ".archgate"), { recursive: true }); + // Write a metadata file with NO token (new format) + await Bun.write( + join(tempDir, ".archgate", "credentials"), + JSON.stringify({ github_user: "testuser", created_at: "2026-01-15" }) + ); + + // Without git credential helper returning creds, result should be null + const result = await loadCredentials(); + // On CI without credential helper configured for this host, this is null + if (!result) { + expect(result).toBeNull(); + } }); }); @@ -119,6 +155,26 @@ describe("credential-store", () => { // Should not throw await clearCredentials(); }); + + test("handles legacy metadata file with plaintext token", async () => { + const { clearCredentials } = + await import("../../src/helpers/credential-store"); + + mkdirSync(join(tempDir, ".archgate"), { recursive: true }); + await Bun.write( + join(tempDir, ".archgate", "credentials"), + JSON.stringify({ + token: "ag_beta_legacy", + github_user: "testuser", + created_at: "2026-01-15", + }) + ); + + await clearCredentials(); + + const credPath = join(tempDir, ".archgate", "credentials"); + expect(await Bun.file(credPath).exists()).toBe(false); + }); }); describe("StoredCredentials type", () => { From aa3c7b937d6ef47507482e97a0afd19d4b8cf774 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Mon, 23 Mar 2026 22:39:20 +0100 Subject: [PATCH 2/4] fix: delete legacy plaintext tokens instead of migrating them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When loadCredentials() finds a legacy metadata file with a plaintext token, it now deletes the file and returns null — prompting the user to run `archgate login` again. No migration attempt is made. Also simplifies clearCredentials() to just delete the file without parsing it for legacy token fields. --- src/helpers/credential-store.ts | 72 ++++++++------------------ tests/helpers/credential-store.test.ts | 17 +++--- 2 files changed, 28 insertions(+), 61 deletions(-) diff --git a/src/helpers/credential-store.ts b/src/helpers/credential-store.ts index 07154aef..ca76e972 100644 --- a/src/helpers/credential-store.ts +++ b/src/helpers/credential-store.ts @@ -30,11 +30,11 @@ export interface StoredCredentials { created_at: string; } -/** Metadata file shape. Legacy files may have a `token` field (auto-migrated). */ +/** Metadata file shape (non-sensitive only). */ interface CredentialMetadata { github_user: string; created_at: string; - /** @deprecated Auto-migrated to git credential manager on load. */ + /** @deprecated Legacy field — presence triggers re-login prompt. */ token?: string; } @@ -202,9 +202,9 @@ export async function saveCredentials( /** * Load stored archgate credentials, or null if none exist. * - * Token is always read from the git credential manager. If a legacy metadata - * file contains a plaintext token, it is auto-migrated to the credential - * manager and scrubbed from disk. + * Token is always read from the git credential manager — never from disk. + * If a legacy metadata file contains a plaintext token, it is deleted and + * the user is asked to run `archgate login` again. */ export async function loadCredentials(): Promise { // Git credential manager is the authoritative token source — check it first @@ -212,10 +212,21 @@ export async function loadCredentials(): Promise { const gitCreds = await gitCredentialFill(); const metadata = await readMetadata(); - if (gitCreds) { - // Scrub legacy plaintext token from metadata file if present. - if (metadata?.token) await writeMetadata(metadata); + // If the metadata file contains a legacy plaintext token, delete it + // regardless of whether git creds exist — plaintext tokens must not persist. + if (metadata?.token) { + logWarn( + "Plaintext token found in credentials file — removing it.", + "Run `archgate login` to re-authenticate." + ); + unlinkSync(metadataPath()); + logDebug("Legacy credentials file with plaintext token removed"); + // Even if git creds exist, force re-login so the metadata file is + // recreated cleanly without a token field. + return null; + } + if (gitCreds) { return { token: gitCreds.password, github_user: gitCreds.username, @@ -223,41 +234,12 @@ export async function loadCredentials(): Promise { }; } - // No git creds — attempt to migrate a legacy plaintext token. - if (metadata?.token) { - logWarn("Migrating plaintext token to git credential manager..."); - const migrated = await gitCredentialApprove( - metadata.github_user, - metadata.token - ); - if (migrated) { - const verified = await gitCredentialFill(); - if (verified) { - logDebug("Legacy token migrated to git credential manager"); - const { token } = metadata; - await writeMetadata(metadata); - return { - token, - github_user: metadata.github_user, - created_at: metadata.created_at, - }; - } - } - logWarn( - "Could not migrate token to git credential manager.", - "Your credential helper may not be configured.", - "Run `archgate login refresh` to re-authenticate." - ); - return null; - } - return null; } /** * Remove stored credentials (logout). - * Clears both the git credential manager and the metadata file, - * including any legacy plaintext tokens. + * Clears both the git credential manager and the metadata file. */ export async function clearCredentials(): Promise { const gitCreds = await gitCredentialFill(); @@ -266,19 +248,7 @@ export async function clearCredentials(): Promise { logDebug("Token removed from git credential manager"); } - const file = Bun.file(metadataPath()); - if (await file.exists()) { - try { - const metadata = (await file.json()) as CredentialMetadata; - if (metadata.token && metadata.github_user) { - await gitCredentialReject(metadata.github_user, metadata.token); - logDebug( - "Legacy plaintext token also rejected from credential manager" - ); - } - } catch { - // Metadata file is invalid — just delete it - } + if (await Bun.file(metadataPath()).exists()) { unlinkSync(metadataPath()); logDebug("Credentials file removed"); } diff --git a/tests/helpers/credential-store.test.ts b/tests/helpers/credential-store.test.ts index 168bf1b1..48ace5af 100644 --- a/tests/helpers/credential-store.test.ts +++ b/tests/helpers/credential-store.test.ts @@ -86,13 +86,14 @@ describe("credential-store", () => { expect(result).toBeNull(); }); - test("attempts migration when metadata file has legacy plaintext token", async () => { + test("returns null and deletes file when metadata has legacy plaintext token", async () => { const { loadCredentials } = await import("../../src/helpers/credential-store"); + const credPath = join(tempDir, ".archgate", "credentials"); mkdirSync(join(tempDir, ".archgate"), { recursive: true }); await Bun.write( - join(tempDir, ".archgate", "credentials"), + credPath, JSON.stringify({ token: "ag_beta_fallback", github_user: "testuser", @@ -100,15 +101,11 @@ describe("credential-store", () => { }) ); - // On test systems without a git credential helper, migration will fail - // and loadCredentials returns null (no insecure plaintext fallback) + // Legacy plaintext tokens are rejected — user must re-login. const result = await loadCredentials(); - // Result depends on whether git credential helper is configured. - // On CI/test systems it's typically null (migration fails gracefully). - if (result) { - expect(result.github_user).toBe("testuser"); - expect(result.token).toBeTruthy(); - } + expect(result).toBeNull(); + // The legacy credentials file should be deleted. + expect(await Bun.file(credPath).exists()).toBe(false); }); test("does not return credentials from plaintext file when git creds fail", async () => { From 7ac62df1c7d2986835388196c37d53a5bdb047e7 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Mon, 23 Mar 2026 22:44:53 +0100 Subject: [PATCH 3/4] refactor: remove metadata file entirely, use only git credential manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ~/.archgate/credentials metadata file was the root cause of the upgrade-loses-login bug and the only place plaintext tokens could exist. - Remove all metadata file read/write logic (writeMetadata, readMetadata) - Remove created_at from StoredCredentials interface - loadCredentials() now only calls git credential fill — no disk I/O - saveCredentials() now only calls git credential approve — no disk I/O - Legacy metadata files are deleted on load/save/clear with a re-login prompt - Update login status to show just the username (from git credential fill) - No docs changes needed — docs don't reference the credentials file --- src/commands/login.ts | 4 +- src/helpers/credential-store.ts | 126 +++++++++---------------- src/helpers/login-flow.ts | 6 +- tests/helpers/auth.test.ts | 57 ++++------- tests/helpers/credential-store.test.ts | 105 ++++++--------------- 5 files changed, 93 insertions(+), 205 deletions(-) diff --git a/src/commands/login.ts b/src/commands/login.ts index dd4b0aa3..7c7eb02f 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -47,9 +47,7 @@ export function registerLoginCommand(program: Command) { .action(async () => { const creds = await loadCredentials(); if (creds) { - console.log( - `Logged in as ${styleText("bold", creds.github_user)} (since ${creds.created_at})` - ); + console.log(`Logged in as ${styleText("bold", creds.github_user)}.`); } else { console.log("Not logged in. Run `archgate login` to authenticate."); } diff --git a/src/helpers/credential-store.ts b/src/helpers/credential-store.ts index ca76e972..597ab590 100644 --- a/src/helpers/credential-store.ts +++ b/src/helpers/credential-store.ts @@ -1,22 +1,22 @@ /** * credential-store.ts — Secure credential storage using git's native credential helpers. * - * Tokens are stored in the OS credential manager (macOS Keychain, Windows - * Credential Manager, libsecret) via `git credential approve/fill/reject`. - * A lightweight JSON metadata file at ~/.archgate/credentials stores only - * non-sensitive data (github_user, created_at). Tokens are NEVER written - * to disk in plaintext. + * Tokens are stored exclusively in the OS credential manager (macOS Keychain, + * Windows Credential Manager, libsecret) via `git credential approve/fill/reject`. + * Nothing is written to disk — no metadata files, no plaintext tokens. + * + * The `username` field in the git credential protocol carries the GitHub username, + * and the `password` field carries the archgate plugin token. * * @see https://git-scm.com/docs/git-credential */ -import { chmodSync, unlinkSync } from "node:fs"; +import { unlinkSync } from "node:fs"; import { logDebug, logWarn } from "./log"; -import { internalPath, createPathIfNotExists } from "./paths"; +import { internalPath } from "./paths"; const CREDENTIAL_HOST = "plugins.archgate.dev"; -const METADATA_FILE = "credentials"; const CREDENTIAL_TIMEOUT_MS = 3_000; /** Build env for git credential commands at call time (not import time). */ @@ -27,15 +27,6 @@ function gitCredentialEnv(): Record { export interface StoredCredentials { token: string; github_user: string; - created_at: string; -} - -/** Metadata file shape (non-sensitive only). */ -interface CredentialMetadata { - github_user: string; - created_at: string; - /** @deprecated Legacy field — presence triggers re-login prompt. */ - token?: string; } // --------------------------------------------------------------------------- @@ -115,38 +106,26 @@ async function gitCredentialReject( } // --------------------------------------------------------------------------- -// Metadata helpers +// Legacy metadata file cleanup // --------------------------------------------------------------------------- -function metadataPath(): string { - return internalPath(METADATA_FILE); -} - -async function readMetadata(): Promise { - const file = Bun.file(metadataPath()); - if (!(await file.exists())) return null; - try { - const data = (await file.json()) as CredentialMetadata; - return data.github_user ? data : null; - } catch { - logDebug("Failed to parse credentials file"); - return null; - } +/** Path to the legacy metadata file (~/.archgate/credentials). */ +function legacyMetadataPath(): string { + return internalPath("credentials"); } -async function writeMetadata(metadata: CredentialMetadata): Promise { - createPathIfNotExists(internalPath()); - const filePath = metadataPath(); - const clean: CredentialMetadata = { - github_user: metadata.github_user, - created_at: metadata.created_at, - }; - await Bun.write(filePath, JSON.stringify(clean, null, 2) + "\n"); - try { - chmodSync(filePath, 0o600); - } catch { - // chmod may fail on Windows — NTFS uses ACLs instead +/** + * Delete the legacy ~/.archgate/credentials file if it exists. + * Returns true if a file was found and deleted. + */ +async function cleanupLegacyMetadata(): Promise { + const file = Bun.file(legacyMetadataPath()); + if (await file.exists()) { + unlinkSync(legacyMetadataPath()); + logDebug("Legacy credentials metadata file removed"); + return true; } + return false; } // --------------------------------------------------------------------------- @@ -157,16 +136,18 @@ const CREDENTIAL_HELPER_HINT = "Run `git config --global credential.helper` to check your configuration."; /** - * Persist archgate credentials securely. + * Persist archgate credentials in the OS credential manager. * - * Token goes to the git credential manager; only non-sensitive metadata is - * written to disk. A verification round-trip confirms the token was actually - * persisted — `git credential approve` exits 0 even without a configured - * helper, silently storing nothing. + * A verification round-trip (`git credential fill`) confirms the token was + * actually persisted — `git credential approve` exits 0 even without a + * configured helper, silently storing nothing. */ export async function saveCredentials( credentials: StoredCredentials ): Promise { + // Clean up any legacy metadata file from previous versions. + await cleanupLegacyMetadata(); + const stored = await gitCredentialApprove( credentials.github_user, credentials.token @@ -191,55 +172,36 @@ export async function saveCredentials( CREDENTIAL_HELPER_HINT ); } - - await writeMetadata({ - github_user: credentials.github_user, - created_at: credentials.created_at, - }); - logDebug("Credentials metadata saved to", metadataPath()); } /** - * Load stored archgate credentials, or null if none exist. + * Load stored archgate credentials from the OS credential manager. + * Returns null if no credentials are stored. * - * Token is always read from the git credential manager — never from disk. - * If a legacy metadata file contains a plaintext token, it is deleted and - * the user is asked to run `archgate login` again. + * If a legacy ~/.archgate/credentials file exists, it is deleted and + * the user is asked to re-login. */ export async function loadCredentials(): Promise { - // Git credential manager is the authoritative token source — check it first - // so credentials survive even if the metadata file is gone (e.g. after upgrade). - const gitCreds = await gitCredentialFill(); - const metadata = await readMetadata(); - - // If the metadata file contains a legacy plaintext token, delete it - // regardless of whether git creds exist — plaintext tokens must not persist. - if (metadata?.token) { + // Delete legacy metadata file — force re-login for a clean slate. + const hadLegacy = await cleanupLegacyMetadata(); + if (hadLegacy) { logWarn( - "Plaintext token found in credentials file — removing it.", + "Legacy credentials file removed.", "Run `archgate login` to re-authenticate." ); - unlinkSync(metadataPath()); - logDebug("Legacy credentials file with plaintext token removed"); - // Even if git creds exist, force re-login so the metadata file is - // recreated cleanly without a token field. return null; } + const gitCreds = await gitCredentialFill(); if (gitCreds) { - return { - token: gitCreds.password, - github_user: gitCreds.username, - created_at: metadata?.created_at ?? "", - }; + return { token: gitCreds.password, github_user: gitCreds.username }; } - return null; } /** * Remove stored credentials (logout). - * Clears both the git credential manager and the metadata file. + * Clears the OS credential manager and any legacy metadata file. */ export async function clearCredentials(): Promise { const gitCreds = await gitCredentialFill(); @@ -247,9 +209,5 @@ export async function clearCredentials(): Promise { await gitCredentialReject(gitCreds.username, gitCreds.password); logDebug("Token removed from git credential manager"); } - - if (await Bun.file(metadataPath()).exists()) { - unlinkSync(metadataPath()); - logDebug("Credentials file removed"); - } + await cleanupLegacyMetadata(); } diff --git a/src/helpers/login-flow.ts b/src/helpers/login-flow.ts index 900d33a1..1d703d18 100644 --- a/src/helpers/login-flow.ts +++ b/src/helpers/login-flow.ts @@ -85,11 +85,7 @@ export async function runLoginFlow( archgateToken = result; } - await saveCredentials({ - token: archgateToken, - github_user: githubUser, - created_at: new Date().toISOString().split("T")[0], - }); + await saveCredentials({ token: archgateToken, github_user: githubUser }); logInfo( `Authenticated as ${styleText("bold", githubUser)}. Plugin access is now available.` diff --git a/tests/helpers/auth.test.ts b/tests/helpers/auth.test.ts index 615fafd2..a56da959 100644 --- a/tests/helpers/auth.test.ts +++ b/tests/helpers/auth.test.ts @@ -39,26 +39,18 @@ describe("auth", () => { }); describe("saveCredentials / loadCredentials", () => { - test("saves metadata without token and round-trips via git credential manager", async () => { - const { saveCredentials, loadCredentials } = + test("does not write any file to disk", async () => { + const { saveCredentials } = await import("../../src/helpers/credential-store"); await saveCredentials({ token: "ag_beta_abc123", github_user: "testuser", - created_at: "2026-01-15", }); - // Metadata file must not contain the token + // No credentials file should exist — everything is in git credential manager. const credPath = join(tempDir, ".archgate", "credentials"); - const metadata = await Bun.file(credPath).json(); - expect(metadata.token).toBeUndefined(); - expect(metadata.github_user).toBe("testuser"); - - // With isolated git config (no credential helper), loadCredentials - // returns null because the token cannot be retrieved from the OS. - const loaded = await loadCredentials(); - expect(loaded).toBeNull(); + expect(await Bun.file(credPath).exists()).toBe(false); }); test("returns null when no credentials exist anywhere", async () => { @@ -69,54 +61,41 @@ describe("auth", () => { expect(result).toBeNull(); }); - test("returns null when credentials file is invalid JSON", async () => { - const { loadCredentials } = - await import("../../src/helpers/credential-store"); - - const credPath = join(tempDir, ".archgate", "credentials"); - const { mkdirSync } = await import("node:fs"); - mkdirSync(join(tempDir, ".archgate"), { recursive: true }); - await Bun.write(credPath, "not-json"); - - const result = await loadCredentials(); - expect(result).toBeNull(); - }); - - test("returns null when credentials file is missing required fields", async () => { + test("returns null and deletes legacy credentials file", async () => { const { loadCredentials } = await import("../../src/helpers/credential-store"); const credPath = join(tempDir, ".archgate", "credentials"); const { mkdirSync } = await import("node:fs"); mkdirSync(join(tempDir, ".archgate"), { recursive: true }); - await Bun.write(credPath, JSON.stringify({ token: "abc" })); + await Bun.write( + credPath, + JSON.stringify({ token: "abc", github_user: "old" }) + ); const result = await loadCredentials(); expect(result).toBeNull(); + expect(await Bun.file(credPath).exists()).toBe(false); }); }); describe("clearCredentials", () => { - test("removes credentials file", async () => { - const { saveCredentials, clearCredentials, loadCredentials } = + test("clears git creds and removes legacy file", async () => { + const { clearCredentials } = await import("../../src/helpers/credential-store"); - await saveCredentials({ - token: "ag_beta_abc123", - github_user: "testuser", - created_at: "2026-01-15", - }); + // Create a legacy file + const { mkdirSync } = await import("node:fs"); + mkdirSync(join(tempDir, ".archgate"), { recursive: true }); + const credPath = join(tempDir, ".archgate", "credentials"); + await Bun.write(credPath, "{}"); await clearCredentials(); - const credPath = join(tempDir, ".archgate", "credentials"); expect(await Bun.file(credPath).exists()).toBe(false); - - const loaded = await loadCredentials(); - expect(loaded).toBeNull(); }); - test("does not throw when no credentials file exists", async () => { + test("does not throw when no credentials exist", async () => { const { clearCredentials } = await import("../../src/helpers/credential-store"); diff --git a/tests/helpers/credential-store.test.ts b/tests/helpers/credential-store.test.ts index 48ace5af..50a7e61a 100644 --- a/tests/helpers/credential-store.test.ts +++ b/tests/helpers/credential-store.test.ts @@ -30,63 +30,52 @@ describe("credential-store", () => { }); describe("saveCredentials", () => { - test("writes metadata file WITHOUT token to ~/.archgate/credentials", async () => { + test("does not write any metadata file to disk", async () => { const { saveCredentials } = await import("../../src/helpers/credential-store"); await saveCredentials({ token: "ag_beta_abc123", github_user: "testuser", - created_at: "2026-01-15", }); + // No credentials file should be written — everything is in git credential manager. const credPath = join(tempDir, ".archgate", "credentials"); - const file = Bun.file(credPath); - expect(await file.exists()).toBe(true); - - const data = await file.json(); - expect(data.github_user).toBe("testuser"); - expect(data.created_at).toBe("2026-01-15"); - // Token must NOT be present in the metadata file - expect(data.token).toBeUndefined(); - }); - }); - - describe("loadCredentials", () => { - test("returns null when no credentials file and no git creds exist", async () => { - const { loadCredentials } = - await import("../../src/helpers/credential-store"); - - const result = await loadCredentials(); - expect(result).toBeNull(); + expect(await Bun.file(credPath).exists()).toBe(false); }); - test("returns null when credentials file is invalid JSON", async () => { - const { loadCredentials } = + test("cleans up legacy metadata file on save", async () => { + const { saveCredentials } = await import("../../src/helpers/credential-store"); + // Create a legacy metadata file mkdirSync(join(tempDir, ".archgate"), { recursive: true }); - await Bun.write(join(tempDir, ".archgate", "credentials"), "not-json"); + const credPath = join(tempDir, ".archgate", "credentials"); + await Bun.write( + credPath, + JSON.stringify({ github_user: "old", created_at: "2025-01-01" }) + ); - const result = await loadCredentials(); - expect(result).toBeNull(); + await saveCredentials({ + token: "ag_beta_abc123", + github_user: "testuser", + }); + + // Legacy file should be removed. + expect(await Bun.file(credPath).exists()).toBe(false); }); + }); - test("returns null when github_user is missing", async () => { + describe("loadCredentials", () => { + test("returns null when no credentials exist anywhere", async () => { const { loadCredentials } = await import("../../src/helpers/credential-store"); - mkdirSync(join(tempDir, ".archgate"), { recursive: true }); - await Bun.write( - join(tempDir, ".archgate", "credentials"), - JSON.stringify({ created_at: "2026-01-01" }) - ); - const result = await loadCredentials(); expect(result).toBeNull(); }); - test("returns null and deletes file when metadata has legacy plaintext token", async () => { + test("returns null and deletes legacy metadata file", async () => { const { loadCredentials } = await import("../../src/helpers/credential-store"); @@ -95,56 +84,29 @@ describe("credential-store", () => { await Bun.write( credPath, JSON.stringify({ - token: "ag_beta_fallback", + token: "ag_beta_legacy", github_user: "testuser", created_at: "2026-01-15", }) ); - // Legacy plaintext tokens are rejected — user must re-login. + // Legacy file triggers deletion and returns null (re-login required). const result = await loadCredentials(); expect(result).toBeNull(); - // The legacy credentials file should be deleted. expect(await Bun.file(credPath).exists()).toBe(false); }); - test("does not return credentials from plaintext file when git creds fail", async () => { + test("returns null when no git creds and no legacy file", async () => { const { loadCredentials } = await import("../../src/helpers/credential-store"); - mkdirSync(join(tempDir, ".archgate"), { recursive: true }); - // Write a metadata file with NO token (new format) - await Bun.write( - join(tempDir, ".archgate", "credentials"), - JSON.stringify({ github_user: "testuser", created_at: "2026-01-15" }) - ); - - // Without git credential helper returning creds, result should be null + // With isolated git config (no credential helper), returns null. const result = await loadCredentials(); - // On CI without credential helper configured for this host, this is null - if (!result) { - expect(result).toBeNull(); - } + expect(result).toBeNull(); }); }); describe("clearCredentials", () => { - test("removes metadata file", async () => { - const { saveCredentials, clearCredentials } = - await import("../../src/helpers/credential-store"); - - await saveCredentials({ - token: "ag_beta_abc123", - github_user: "testuser", - created_at: "2026-01-15", - }); - - await clearCredentials(); - - const credPath = join(tempDir, ".archgate", "credentials"); - expect(await Bun.file(credPath).exists()).toBe(false); - }); - test("does not throw when no credentials exist", async () => { const { clearCredentials } = await import("../../src/helpers/credential-store"); @@ -153,23 +115,19 @@ describe("credential-store", () => { await clearCredentials(); }); - test("handles legacy metadata file with plaintext token", async () => { + test("cleans up legacy metadata file", async () => { const { clearCredentials } = await import("../../src/helpers/credential-store"); mkdirSync(join(tempDir, ".archgate"), { recursive: true }); + const credPath = join(tempDir, ".archgate", "credentials"); await Bun.write( - join(tempDir, ".archgate", "credentials"), - JSON.stringify({ - token: "ag_beta_legacy", - github_user: "testuser", - created_at: "2026-01-15", - }) + credPath, + JSON.stringify({ github_user: "testuser", created_at: "2026-01-15" }) ); await clearCredentials(); - const credPath = join(tempDir, ".archgate", "credentials"); expect(await Bun.file(credPath).exists()).toBe(false); }); }); @@ -177,7 +135,6 @@ describe("credential-store", () => { describe("StoredCredentials type", () => { test("interface has expected shape", async () => { const mod = await import("../../src/helpers/credential-store"); - // Verify the module exports the expected functions expect(typeof mod.saveCredentials).toBe("function"); expect(typeof mod.loadCredentials).toBe("function"); expect(typeof mod.clearCredentials).toBe("function"); From 4f4b84d1719ea6a45bd29a7e36fef390d7d3c5f3 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Mon, 23 Mar 2026 22:55:35 +0100 Subject: [PATCH 4/4] fix(ci): use bun run test in validate script for consistent timeout The validate script called `bun test` directly, bypassing the --timeout 60000 flag from the test script. This caused the init idempotency integration test to flake at the default 5s timeout on CI. --- package.json | 2 +- tests/integration/init.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index e0edee47..ecfe52cd 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "test": "bun test --timeout 60000", "test:watch": "bun test --watch --timeout 60000", "typecheck": "tsc --build", - "validate": "bun run lint && bun run typecheck && bun run format:check && bun test && bun run check && bun run build:check" + "validate": "bun run lint && bun run typecheck && bun run format:check && bun run test && bun run check && bun run build:check" }, "devDependencies": { "@commander-js/extra-typings": "14.0.0", diff --git a/tests/integration/init.test.ts b/tests/integration/init.test.ts index 5098aa17..2cebe76c 100644 --- a/tests/integration/init.test.ts +++ b/tests/integration/init.test.ts @@ -8,7 +8,7 @@ import { runCli, createTempProject } from "./cli-harness"; let tempDir: string; let fakeHome: string; -/** Env overrides to isolate from real credentials in ~/.archgate/credentials */ +/** Env overrides to isolate from real ~/.archgate/ state */ function isolatedEnv(): Record { return { HOME: fakeHome, USERPROFILE: fakeHome }; }