Skip to content

fix(cli): surface actionable re-auth message on revoked cloud API key #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andrei-hasna wants to merge 1 commit into
base: main
Choose a base branch
from
+135 −20
Open

fix(cli): surface actionable re-auth message on revoked cloud API key #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 30 additions & 20 deletions src/cli/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { ensureProjectDashboardStandardConfig } from "../lib/project-dashboard-s
import { ensureGlobalAgentRulesStandardConfig } from "../lib/global-agent-rules-standard.js";
import { ensureDangerousOperationGuardStandardConfig } from "../lib/dangerous-operation-guard-standard.js";
import { getConfigsStatus } from "../status.js";
import { resolveConfigStore, isCloudMode, type ConfigStore } from "../data/config-store.js";
import { resolveConfigStore, isCloudMode, formatCliError, type ConfigStore } from "../data/config-store.js";
import { DEFAULT_LIST_LIMIT, paginate, parseLimit, truncateMiddle, truncateText } from "../lib/compact-output.js";
import type { Config, ConfigAgent, ConfigCategory, ConfigFormat, ConfigKind, Profile, ProfileSelector, ProfileVariables } from "../types/index.js";

Expand Down Expand Up @@ -304,7 +304,7 @@ program
console.log(chalk.dim("─".repeat(60)));
printLine(c.content);
} catch (e) {
console.error(chalk.red(e instanceof Error ? e.message : String(e)));
console.error(chalk.red(formatCliError(e)));
process.exit(1);
}
});
Expand Down Expand Up @@ -361,7 +361,7 @@ program
if (opts.json) { printJson({ deleted: true, id: config.id, slug: config.slug }); return; }
console.log(chalk.green("✓") + ` Deleted: ${chalk.bold(config.name)} ${chalk.dim(`(${config.slug})`)}`);
} catch (e) {
console.error(chalk.red(e instanceof Error ? e.message : String(e)));
console.error(chalk.red(formatCliError(e)));
process.exit(1);
}
});
Expand All @@ -386,7 +386,7 @@ program
console.log(` ${outputStatus} ${output.path} ${chalk.dim(`[${output.agent}/${output.transform}] (${outputChange})`)}`);
}
} catch (e) {
console.error(chalk.red(e instanceof Error ? e.message : String(e)));
console.error(chalk.red(formatCliError(e)));
process.exit(1);
}
});
Expand Down Expand Up @@ -418,7 +418,7 @@ program
}
console.log(chalk.dim(`${drifted}/${configs.length} drifted`));
} catch (e) {
console.error(chalk.red(e instanceof Error ? e.message : String(e)));
console.error(chalk.red(formatCliError(e)));
process.exit(1);
}
});
Expand Down Expand Up @@ -635,7 +635,7 @@ profileCmd.command("show <id>").description("Show profile and its configs")
if (page.has_more) {
console.log(chalk.dim(`Showing ${page.items.length} of ${page.total}. Next: configs profile show ${id} --cursor ${page.next_cursor} --limit ${page.limit}`));
}
} catch (e) { console.error(chalk.red(e instanceof Error ? e.message : String(e))); process.exit(1); }
} catch (e) { console.error(chalk.red(formatCliError(e))); process.exit(1); }
});

profileCmd.command("add <profile> <config>").description("Add a config to a profile").action(async (profile, config) => {
Expand All @@ -644,7 +644,7 @@ profileCmd.command("add <profile> <config>").description("Add a config to a prof
const c = await store.getConfig(config);
await store.addConfigToProfile(profile, c.id);
console.log(chalk.green("✓") + ` Added ${c.slug} to profile ${profile}`);
} catch (e) { console.error(chalk.red(e instanceof Error ? e.message : String(e))); process.exit(1); }
} catch (e) { console.error(chalk.red(formatCliError(e))); process.exit(1); }
});

profileCmd.command("remove <profile> <config>").description("Remove a config from a profile").action(async (profile, config) => {
Expand All @@ -653,7 +653,7 @@ profileCmd.command("remove <profile> <config>").description("Remove a config fro
const c = await store.getConfig(config);
await store.removeConfigFromProfile(profile, c.id);
console.log(chalk.green("✓") + ` Removed ${c.slug} from profile ${profile}`);
} catch (e) { console.error(chalk.red(e instanceof Error ? e.message : String(e))); process.exit(1); }
} catch (e) { console.error(chalk.red(formatCliError(e))); process.exit(1); }
});

profileCmd.command("apply [id]").description("Apply all configs in a profile to disk")
Expand Down Expand Up @@ -681,7 +681,7 @@ profileCmd.command("apply [id]").description("Apply all configs in a profile to
if (r.changed) changed++;
}
console.log(chalk.dim(`\n${changed}/${results.length} changed (${selected.slug} on ${machine.hostname} ${machine.os_family}/${machine.arch})`));
} catch (e) { console.error(chalk.red(e instanceof Error ? e.message : String(e))); process.exit(1); }
} catch (e) { console.error(chalk.red(formatCliError(e))); process.exit(1); }
});

profileCmd.command("resolve").description("Resolve the matching machine-aware profile")
Expand Down Expand Up @@ -711,7 +711,7 @@ profileCmd.command("delete <id>").description("Delete a profile").action(async (
const p = await store.getProfile(id);
await store.deleteProfile(p.id);
console.log(chalk.green("✓") + ` Deleted profile: ${p.name}`);
} catch (e) { console.error(chalk.red(e instanceof Error ? e.message : String(e))); process.exit(1); }
} catch (e) { console.error(chalk.red(formatCliError(e))); process.exit(1); }
});

// ── session ──────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -768,7 +768,7 @@ sessionCmd.command("plan")
}
console.log(chalk.dim("Dry run only. No files were written."));
} catch (e) {
console.error(chalk.red(e instanceof Error ? e.message : String(e)));
console.error(chalk.red(formatCliError(e)));
process.exit(1);
}
});
Expand Down Expand Up @@ -832,7 +832,7 @@ sessionCmd.command("apply")
process.exitCode = 1;
}
} catch (e) {
console.error(chalk.red(e instanceof Error ? e.message : String(e)));
console.error(chalk.red(formatCliError(e)));
process.exit(1);
}
});
Expand All @@ -854,7 +854,7 @@ snapshotCmd.command("list <config>").description("List snapshots for a config")
console.log(` v${s.version} ${chalk.dim(s.created_at)} ${chalk.dim(s.id)}`);
}
pageFooter(`configs snapshot list ${configId}`, page, "Use `configs snapshot show <id>` to print snapshot content.");
} catch (e) { console.error(chalk.red(e instanceof Error ? e.message : String(e))); process.exit(1); }
} catch (e) { console.error(chalk.red(formatCliError(e))); process.exit(1); }
});

snapshotCmd.command("show <id>").description("Show a snapshot's content").action(async (id) => {
Expand All @@ -870,7 +870,7 @@ snapshotCmd.command("restore <config> <snapshot-id>").description("Restore a con
if (!snap) { console.error(chalk.red("Snapshot not found: " + snapId)); process.exit(1); }
await store.updateConfig(configId, { content: snap.content });
console.log(chalk.green("✓") + ` Restored ${configId} to snapshot v${snap.version}`);
} catch (e) { console.error(chalk.red(e instanceof Error ? e.message : String(e))); process.exit(1); }
} catch (e) { console.error(chalk.red(formatCliError(e))); process.exit(1); }
});

// ── template ──────────────────────────────────────────────────────────────────
Expand All @@ -884,7 +884,7 @@ templateCmd.command("vars <id>").description("Show template variables").action(a
for (const v of vars) {
console.log(` ${chalk.cyan("{{" + v.name + "}}")}${v.description ? chalk.dim(" — " + v.description) : ""}`);
}
} catch (e) { console.error(chalk.red(e instanceof Error ? e.message : String(e))); process.exit(1); }
} catch (e) { console.error(chalk.red(formatCliError(e))); process.exit(1); }
});

templateCmd.command("render <id>")
Expand Down Expand Up @@ -936,7 +936,7 @@ templateCmd.command("render <id>")
console.log(rendered);
}
} catch (e) {
console.error(chalk.red(e instanceof Error ? e.message : String(e)));
console.error(chalk.red(formatCliError(e)));
process.exit(1);
}
});
Expand Down Expand Up @@ -1113,7 +1113,7 @@ mcpCmd.command("install")
console.log(chalk.green("✓") + " Installed into Antigravity");
}
} catch (e) {
console.error(chalk.red(`✗ Failed to install into ${target}: ${e instanceof Error ? e.message : String(e)}`));
console.error(chalk.red(`✗ Failed to install into ${target}: ${formatCliError(e)}`));
}
}
});
Expand Down Expand Up @@ -1373,7 +1373,7 @@ program
console.log(chalk.dim(`\n${diffs} difference(s)`));
}
} catch (e) {
console.error(chalk.red(e instanceof Error ? e.message : String(e)));
console.error(chalk.red(formatCliError(e)));
process.exit(1);
}
});
Expand Down Expand Up @@ -1651,7 +1651,7 @@ program
}
}
} catch (e) {
console.error(chalk.red("Failed to check for updates: " + (e instanceof Error ? e.message : String(e))));
console.error(chalk.red("Failed to check for updates: " + (formatCliError(e))));
}
});

Expand All @@ -1673,4 +1673,14 @@ program

program.version(pkg.version).name("instructions");
registerEventsCommands(program, { source: "configs" });
program.parse(process.argv);

// Use parseAsync so async action rejections are awaited and routed through a
// single top-level handler. With the synchronous `program.parse`, a rejected
// async action (e.g. a revoked-API-key CloudHttpError on `list`/`add`) surfaced
// as a raw unhandled-promise stack trace. formatCliError turns auth failures
// into an actionable re-auth message; commands with their own try/catch still
// handle and exit before reaching here.
program.parseAsync(process.argv).catch((e) => {
console.error(chalk.red(formatCliError(e)));
process.exit(1);
});
69 changes: 69 additions & 0 deletions src/data/config-store.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {
CloudConfigStore,
CloudHttpError,
LocalConfigStore,
formatCliError,
isCloudAuthError,
isCloudMode,
resolveCloudConfig,
resolveConfigStore,
Expand Down Expand Up @@ -175,3 +177,70 @@ describe("CloudConfigStore CRUD mapping", () => {
expect(configs).toHaveLength(1);
});
});

describe("revoked / invalid API key handling", () => {
const AUTH_ENV = {
HASNA_INSTRUCTIONS_API_URL: "https://instructions.hasna.xyz",
HASNA_INSTRUCTIONS_API_KEY: "revoked-key",
};

test("isCloudAuthError only matches 401/403 CloudHttpError", () => {
expect(isCloudAuthError(new CloudHttpError(401, "API key has been revoked"))).toBe(true);
expect(isCloudAuthError(new CloudHttpError(403, "forbidden"))).toBe(true);
expect(isCloudAuthError(new CloudHttpError(500, "boom"))).toBe(false);
expect(isCloudAuthError(new CloudHttpError(404, "not found"))).toBe(false);
expect(isCloudAuthError(new Error("network down"))).toBe(false);
});

test("formatCliError rewrites a revoked-key error into an actionable re-auth message", () => {
const msg = formatCliError(new CloudHttpError(401, "API key has been revoked"), AUTH_ENV);
expect(msg).toContain("authentication failed");
expect(msg).toContain("API key has been revoked");
expect(msg).toContain("HASNA_INSTRUCTIONS_API_KEY");
expect(msg).toContain("https://instructions.hasna.xyz");
// offers both a re-auth and a local-store fallback path
expect(msg).toContain("export HASNA_INSTRUCTIONS_API_KEY=");
expect(msg).toContain("unset HASNA_INSTRUCTIONS_API_URL HASNA_INSTRUCTIONS_API_KEY");
});

test("formatCliError leaves non-auth errors as plain messages", () => {
expect(formatCliError(new CloudHttpError(500, "internal error"), AUTH_ENV)).toBe("internal error");
expect(formatCliError(new Error("disk full"))).toBe("disk full");
expect(formatCliError("raw string")).toBe("raw string");
});

test("formatCliError omits the generic HTTP fallback note but keeps guidance", () => {
const msg = formatCliError(new CloudHttpError(401, "HTTP 401 on GET /configs"), AUTH_ENV);
expect(msg).not.toContain("Server said:");
expect(msg).toContain("missing, expired, or revoked");
});

test("cloud list path with revoked key surfaces an auth error that formats cleanly", async () => {
const m = mockFetch(() => ({ status: 401, json: { error: "API key has been revoked" } }));
active = m;
const store = new CloudConfigStore(CONFIG);
try {
await store.listConfigs();
throw new Error("expected listConfigs to reject");
} catch (err) {
expect(isCloudAuthError(err)).toBe(true);
const shown = formatCliError(err, AUTH_ENV);
expect(shown).toContain("authentication failed");
expect(shown).toContain("unset HASNA_INSTRUCTIONS_API_URL HASNA_INSTRUCTIONS_API_KEY");
}
});

test("cloud create path with revoked key surfaces an auth error that formats cleanly", async () => {
const m = mockFetch(() => ({ status: 401, json: { error: "API key has been revoked" } }));
active = m;
const store = new CloudConfigStore(CONFIG);
try {
await store.createConfig({ name: "Demo", category: "rules" as never, content: "hello" });
throw new Error("expected createConfig to reject");
} catch (err) {
expect(isCloudAuthError(err)).toBe(true);
const shown = formatCliError(err, AUTH_ENV);
expect(shown).toContain("export HASNA_INSTRUCTIONS_API_KEY=");
}
});
});
36 changes: 36 additions & 0 deletions src/data/config-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,42 @@ export class CloudHttpError extends Error {
const API_URL_ENV = "HASNA_INSTRUCTIONS_API_URL";
const API_KEY_ENV = "HASNA_INSTRUCTIONS_API_KEY";

/**
* True when `err` is a cloud authentication failure — an HTTP 401/403 from the
* `/v1` API, which is what a missing, expired, or revoked bearer key produces.
*/
export function isCloudAuthError(err: unknown): err is CloudHttpError {
return err instanceof CloudHttpError && (err.status === 401 || err.status === 403);
}

/**
* Render an error for CLI/user display. A cloud auth failure (401/403 — e.g. a
* revoked or invalid `HASNA_INSTRUCTIONS_API_KEY`) is rewritten into a clear,
* actionable re-auth message so the operator is not blocked by a raw
* `CloudHttpError`: they can rotate the key or fall back to the local store.
* All other errors fall back to their plain message (unchanged behaviour).
*/
export function formatCliError(err: unknown, env: NodeJS.ProcessEnv = process.env): string {
if (isCloudAuthError(err)) {
const apiUrl = env[API_URL_ENV]?.trim();
const detail = err.message?.trim();
// Only echo the server's own message when it adds signal beyond the generic
// `HTTP <status> on ...` fallback synthesised by CloudConfigStore.request.
const serverNote = detail && !/^HTTP \d+\b/.test(detail) ? ` Server said: ${detail}` : "";
return [
`Instructions cloud API rejected the request (HTTP ${err.status}: authentication failed).`,
serverNote,
` The API key in ${API_KEY_ENV} is missing, expired, or revoked${apiUrl ? ` for ${apiUrl}` : ""}.`,
` To continue, either:`,
` - set a valid key: export ${API_KEY_ENV}=<new-key>`,
` - or use the local store instead: unset ${API_URL_ENV} ${API_KEY_ENV}`,
]
.filter(Boolean)
.join("\n");
}
return err instanceof Error ? err.message : String(err);
}

/**
* Resolve cloud config from the environment.
* - both vars set -> config (api transport: self_hosted / cloud)
Expand Down
Loading