diff --git a/src/cli/commands/api.ts b/src/cli/commands/api.ts index 2bd9da5..6909ea7 100644 --- a/src/cli/commands/api.ts +++ b/src/cli/commands/api.ts @@ -9,6 +9,7 @@ import { parseFieldsEffect, parseHeadersEffect, readBodyFromFileEffect, + sanitizeResponseHeaders, } from "../../core/api"; import { authLoginEffect, getTokenInfoEffect } from "../../core/auth"; import { AuthenticationError, ValidationError } from "../../effect/errors"; @@ -318,7 +319,9 @@ const apiCommand = Command.make( method, status: response.status, status_text: response.statusText, - headers: config.include ? response.headers : undefined, + headers: config.include + ? sanitizeResponseHeaders(response.headers) + : undefined, data: output ?? null, }, apiRequestActions, diff --git a/src/core/api.ts b/src/core/api.ts index 4dd9700..f53799e 100644 --- a/src/core/api.ts +++ b/src/core/api.ts @@ -16,6 +16,60 @@ import { type Environment, envGetEffect, getApiUrl } from "./environment"; // Minimum seconds before expiry to consider token valid for a request const TOKEN_EXPIRY_BUFFER_SECONDS = 30; +// Header names (lowercased) that must be redacted from debug output and +// the --include envelope to prevent leaking tokens, cookies, or secrets. +const SENSITIVE_HEADER_PARTS = [ + "authorization", + "cookie", + "set-cookie", + "token", + "secret", + "api-key", + "apikey", + "x-auth", +] as const; + +function isSensitiveHeader(headerName: string): boolean { + const lower = headerName.toLowerCase(); + return SENSITIVE_HEADER_PARTS.some((part) => lower.includes(part)); +} + +/** + * Return a copy of headers with sensitive values replaced by "[REDACTED]". + */ +export { sanitizeHeaders as sanitizeResponseHeaders }; + +function sanitizeHeaders( + headers: Record, +): Record { + const sanitized: Record = {}; + for (const [key, value] of Object.entries(headers)) { + sanitized[key] = isSensitiveHeader(key) ? "[REDACTED]" : value; + } + return sanitized; +} + +/** + * Redact values whose keys look like they contain secrets. + */ +function redactSensitiveBodyFields(body: string): string { + try { + const parsed = JSON.parse(body); + if (typeof parsed !== "object" || parsed === null) return body; + const redacted: Record = {}; + for (const [key, value] of Object.entries(parsed)) { + const lower = key.toLowerCase(); + const isSensitive = SENSITIVE_HEADER_PARTS.some((part) => + lower.includes(part), + ) || lower.includes("password") || lower.includes("credential"); + redacted[key] = isSensitive ? "[REDACTED]" : value; + } + return JSON.stringify(redacted); + } catch { + return body; + } +} + export type HttpMethod = "GET" | "POST" | "PUT" | "PATCH" | "DELETE"; export interface ApiRequestOptions { @@ -265,13 +319,12 @@ export function apiRequestEffect( if (debug) { console.error(`> ${method} ${url}`); - for (const [key, value] of Object.entries(requestHeaders)) { - const displayValue = - key.toLowerCase() === "authorization" ? "Bearer [REDACTED]" : value; - console.error(`> ${key}: ${displayValue}`); + const sanitizedRequestHeaders = sanitizeHeaders(requestHeaders); + for (const [key, value] of Object.entries(sanitizedRequestHeaders)) { + console.error(`> ${key}: ${value}`); } if (requestBody) { - console.error(`> Body: ${requestBody}`); + console.error(`> Body: ${redactSensitiveBodyFields(requestBody)}`); } console.error(""); } @@ -302,7 +355,8 @@ export function apiRequestEffect( if (debug) { console.error(`< ${response.status} ${response.statusText}`); - for (const [key, value] of Object.entries(responseHeaders)) { + const sanitizedResponseHeaders = sanitizeHeaders(responseHeaders); + for (const [key, value] of Object.entries(sanitizedResponseHeaders)) { console.error(`< ${key}: ${value}`); } console.error(""); @@ -340,7 +394,9 @@ export function apiRequestEffect( // Check for error status codes if (!response.ok) { - const errorMessage = + // Internal message includes the raw server payload for debugging; + // userMessage is a safe, generic description shown to users/agents. + const internalDetail = typeof data === "object" && data !== null ? JSON.stringify(data) : String(data || response.statusText); @@ -349,7 +405,7 @@ export function apiRequestEffect( if (response.status === 401) { return yield* Effect.fail( new AuthenticationError({ - message: `Authentication failed (401): ${errorMessage}`, + message: `Authentication failed (401): ${internalDetail}`, userMessage: "Your session has expired or is invalid. Run 'godaddy auth login' to re-authenticate.", }), @@ -360,7 +416,7 @@ export function apiRequestEffect( if (response.status === 403) { return yield* Effect.fail( new AuthenticationError({ - message: `Access denied (403): ${errorMessage}`, + message: `Access denied (403): ${internalDetail}`, userMessage: "You don't have permission to access this resource. Check your account permissions.", }), @@ -369,7 +425,7 @@ export function apiRequestEffect( return yield* Effect.fail( new NetworkError({ - message: `API error (${response.status}): ${errorMessage}`, + message: `API error (${response.status}): ${internalDetail}`, userMessage: `API request failed with status ${response.status}: ${response.statusText}`, }), ); diff --git a/src/core/auth.ts b/src/core/auth.ts index e03fb60..35020d0 100644 --- a/src/core/auth.ts +++ b/src/core/auth.ts @@ -25,8 +25,21 @@ import { } from "./token-store"; const PORT = 7443; +const AUTH_HOST = "127.0.0.1"; +const AUTH_TIMEOUT_MS = 120_000; const DEFAULT_OAUTH_SCOPES = "apps.app-registry:read apps.app-registry:write"; +/** + * Escape a string for safe embedding in HTML text content. + */ +function escapeHtml(text: string): string { + return text + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """); +} + export interface AuthResult { success: boolean; accessToken?: string; @@ -217,10 +230,11 @@ export function authLoginEffect(options?: { "Content-Type": "text/html", }); res.end( - `

Authentication Failed

${errorMessage}

`, + `

Authentication Failed

${escapeHtml(errorMessage)}

`, ); reject(err); } finally { + clearTimeout(authTimeout); if (server) server.close(); } } else { @@ -234,7 +248,20 @@ export function authLoginEffect(options?: { reject(err); }); - server.listen(PORT, () => { + // Auto-close after timeout if the user never completes the flow + const authTimeout = setTimeout(() => { + if (server) { + server.close(); + server = null; + } + reject( + new Error( + "Login timed out. The authentication flow was not completed.", + ), + ); + }, AUTH_TIMEOUT_MS); + + server.listen(PORT, AUTH_HOST, () => { const actualPort = (server?.address() as import("net").AddressInfo) ?.port; if (!actualPort) { diff --git a/src/effect/layers/keychain-native.ts b/src/effect/layers/keychain-native.ts index e8a3520..7e3c140 100644 --- a/src/effect/layers/keychain-native.ts +++ b/src/effect/layers/keychain-native.ts @@ -288,6 +288,9 @@ function createWindowsKeychain(): KeychainService { return { async setPassword(service, account, password) { + assertSafePSValue(service, "service"); + assertSafePSValue(account, "account"); + assertSafePSValue(password, "password"); // Remove existing entry first (PasswordVault throws on duplicate) const removeScript = ` ${vaultInit} @@ -307,6 +310,8 @@ function createWindowsKeychain(): KeychainService { }, async getPassword(service, account) { + assertSafePSValue(service, "service"); + assertSafePSValue(account, "account"); const script = ` ${vaultInit} try { @@ -325,6 +330,8 @@ function createWindowsKeychain(): KeychainService { }, async deletePassword(service, account) { + assertSafePSValue(service, "service"); + assertSafePSValue(account, "account"); const script = ` ${vaultInit} try { @@ -339,6 +346,7 @@ function createWindowsKeychain(): KeychainService { }, async findCredentials(service) { + assertSafePSValue(service, "service"); const script = ` ${vaultInit} try { @@ -370,9 +378,33 @@ function createWindowsKeychain(): KeychainService { }; } -/** Escape single quotes for PowerShell string literals. */ +/** + * Escape a value for embedding inside PowerShell single-quoted strings. + * + * PowerShell single-quoted strings only interpret '' as a literal '. + * However, we must also strip NUL bytes (which can truncate strings in + * some contexts) and reject values containing characters that could break + * out of the quoting context if the script template is ever changed to + * use double-quotes or here-strings. + */ function escapePS(value: string): string { - return value.replace(/'/g, "''"); + // Strip NUL bytes that could truncate the string + const stripped = value.replace(/\0/g, ""); + // In PowerShell single-quoted strings, only ' needs escaping (as ''). + return stripped.replace(/'/g, "''"); +} + +/** + * Validate that a value is safe for PowerShell interpolation. + * Rejects values containing newlines or control characters that could + * escape the single-line script template. + */ +function assertSafePSValue(value: string, label: string): void { + if (/[\r\n]/.test(value)) { + throw new Error( + `Unsafe ${label} for credential storage: contains newline characters`, + ); + } } // --------------------------------------------------------------------------- diff --git a/src/services/applications.ts b/src/services/applications.ts index aec994d..3a73b22 100644 --- a/src/services/applications.ts +++ b/src/services/applications.ts @@ -175,7 +175,8 @@ export const releaseInput = type({ }); /** - * Extract a user-friendly error message from a GraphQL ClientError + * Extract the internal error detail from a GraphQL ClientError. + * This may include server-side messages and extension codes. */ function extractGraphQLError(err: unknown): string { if (err instanceof ClientError) { @@ -189,6 +190,29 @@ function extractGraphQLError(err: unknown): string { return "An unexpected error occurred"; } +/** + * Return a safe, generic user-facing message for a GraphQL error. + * Avoids leaking internal server details in the CLI JSON envelope. + */ +function safeGraphQLUserMessage(err: unknown): string { + if (err instanceof ClientError) { + const status = err.response.status; + if (status === 401 || status === 403) + return "Authentication failed. Run 'godaddy auth login'."; + if (status === 404) + return "The requested resource was not found."; + if (status && status >= 500) + return "The server encountered an error. Please try again later."; + // For 4xx with GraphQL-level error messages, allow the first message + // through since these are validation-style errors the user can act on. + const graphqlErrors = err.response.errors; + if (graphqlErrors?.length && graphqlErrors[0].message) { + return graphqlErrors[0].message; + } + } + return "An unexpected error occurred"; +} + export function createApplicationEffect( input: typeof applicationInput.infer, { accessToken }: { accessToken: string | null }, @@ -225,7 +249,7 @@ export function createApplicationEffect( catch: (err) => new NetworkError({ message: extractGraphQLError(err), - userMessage: extractGraphQLError(err), + userMessage: safeGraphQLUserMessage(err), }), }); }); @@ -258,7 +282,7 @@ export function updateApplicationEffect( catch: (err) => new NetworkError({ message: extractGraphQLError(err), - userMessage: extractGraphQLError(err), + userMessage: safeGraphQLUserMessage(err), }), }); }); @@ -290,7 +314,7 @@ export function getApplicationEffect( catch: (err) => new NetworkError({ message: extractGraphQLError(err), - userMessage: extractGraphQLError(err), + userMessage: safeGraphQLUserMessage(err), }), }); }); @@ -322,7 +346,7 @@ export function getApplicationAndLatestReleaseEffect( catch: (err) => new NetworkError({ message: extractGraphQLError(err), - userMessage: extractGraphQLError(err), + userMessage: safeGraphQLUserMessage(err), }), }); }); @@ -370,7 +394,7 @@ export function createReleaseEffect( catch: (err) => new NetworkError({ message: extractGraphQLError(err), - userMessage: extractGraphQLError(err), + userMessage: safeGraphQLUserMessage(err), }), }); }); @@ -402,7 +426,7 @@ export function enableApplicationEffect( catch: (err) => new NetworkError({ message: extractGraphQLError(err), - userMessage: extractGraphQLError(err), + userMessage: safeGraphQLUserMessage(err), }), }); }); @@ -434,7 +458,7 @@ export function disableApplicationEffect( catch: (err) => new NetworkError({ message: extractGraphQLError(err), - userMessage: extractGraphQLError(err), + userMessage: safeGraphQLUserMessage(err), }), }); }); @@ -465,7 +489,7 @@ export function listApplicationsEffect({ catch: (err) => new NetworkError({ message: extractGraphQLError(err), - userMessage: extractGraphQLError(err), + userMessage: safeGraphQLUserMessage(err), }), }); }); @@ -497,7 +521,7 @@ export function archiveApplicationEffect( catch: (err) => new NetworkError({ message: extractGraphQLError(err), - userMessage: extractGraphQLError(err), + userMessage: safeGraphQLUserMessage(err), }), }); }); diff --git a/src/services/webhook-events.ts b/src/services/webhook-events.ts index bcc3d48..bb3185a 100644 --- a/src/services/webhook-events.ts +++ b/src/services/webhook-events.ts @@ -95,7 +95,10 @@ export function getWebhookEventsTypesEffect({ return yield* Effect.fail( new AuthenticationError({ message: json.error || "Authentication failed", - userMessage: json.error || "Authentication failed", + userMessage: + response.status === 401 || response.status === 403 + ? "Authentication failed. Run 'godaddy auth login'." + : `Webhook events request failed with status ${response.status}`, }), ); }