Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@insforge/cli",
"version": "0.1.75",
"version": "0.1.76",
"description": "InsForge CLI - Command line tool for InsForge platform",
"type": "module",
"bin": {
Expand Down
212 changes: 212 additions & 0 deletions src/commands/config/apply.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,30 @@ import type * as ErrorsModule from '../../lib/errors.js';

// Per-test we override what /api/metadata returns by reassigning this.
let nextMetadataResponse: unknown = {};
// Stash secret values for env() resolution. Map secret name → value.
const secretsStore: Map<string, string> = new Map();
const ossFetchMock = vi.fn(async (path: string, init?: RequestInit) => {
if (path === '/api/metadata' && (!init || init.method === undefined || init.method === 'GET')) {
return new Response(JSON.stringify(nextMetadataResponse), {
status: 200,
headers: { 'content-type': 'application/json' },
});
}
const secretMatch = path.match(/^\/api\/secrets\/(.+)$/);
if (secretMatch && (!init || init.method === undefined || init.method === 'GET')) {
const key = decodeURIComponent(secretMatch[1]);
const value = secretsStore.get(key);
if (value === undefined) {
// Real ossFetch throws on any non-2xx instead of returning the Response.
// Mirror that: the resolver recovers the "missing" signal from the error
// message because the underlying status is unreachable.
throw new Error(`Secret not found: ${key}`);
}
return new Response(JSON.stringify({ key, value }), {
status: 200,
headers: { 'content-type': 'application/json' },
});
}
return new Response(JSON.stringify({ ok: true }), {
status: 200,
headers: { 'content-type': 'application/json' },
Expand Down Expand Up @@ -75,6 +92,7 @@ let tmp: string;

beforeEach(() => {
vi.clearAllMocks();
secretsStore.clear();
tmp = mkdtempSync(join(tmpdir(), 'insforge-apply-test-'));
});
Comment on lines 93 to 97
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reset nextMetadataResponse in beforeEach to prevent cross-test coupling.

nextMetadataResponse is shared mutable state but isn’t reset per test. Add a reset in beforeEach so future tests can’t accidentally inherit previous metadata.

Suggested patch
 beforeEach(() => {
   vi.clearAllMocks();
   secretsStore.clear();
+  nextMetadataResponse = {};
   tmp = mkdtempSync(join(tmpdir(), 'insforge-apply-test-'));
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
beforeEach(() => {
vi.clearAllMocks();
secretsStore.clear();
tmp = mkdtempSync(join(tmpdir(), 'insforge-apply-test-'));
});
beforeEach(() => {
vi.clearAllMocks();
secretsStore.clear();
nextMetadataResponse = {};
tmp = mkdtempSync(join(tmpdir(), 'insforge-apply-test-'));
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/config/apply.test.ts` around lines 93 - 97, The shared test
state variable nextMetadataResponse is not reset between tests; update the
beforeEach block (the one that calls vi.clearAllMocks(), secretsStore.clear(),
and sets tmp) to also reset nextMetadataResponse (e.g. set to undefined/null or
its initial value) so each test starts with a clean metadata state and cannot
inherit prior test responses.


Expand Down Expand Up @@ -166,3 +184,197 @@ describe('config apply (capability probe)', () => {
rmSync(tmp, { recursive: true, force: true });
});
});

describe('config apply — auth.smtp', () => {
const EMPTY_SMTP_METADATA = {
enabled: false,
host: '',
port: 587,
username: '',
hasPassword: false,
senderEmail: '',
senderName: '',
minIntervalSeconds: 60,
};

it('resolves env() ref and PUTs /api/auth/smtp-config with the actual password', async () => {
nextMetadataResponse = {
auth: { allowedRedirectUrls: [], smtpConfig: EMPTY_SMTP_METADATA },
};
secretsStore.set('SMTP_PASSWORD', 'real-secret-from-store');

const tomlPath = join(tmp, 'insforge.toml');
writeFileSync(
tomlPath,
`[auth.smtp]
enabled = true
host = "smtp.gmail.com"
port = 587
username = "user@gmail.com"
password = "env(SMTP_PASSWORD)"
sender_email = "noreply@app.com"
sender_name = "App"
min_interval_seconds = 60
`,
);

const program = makeProgram();
const docs = await runJson(program, [
'--json',
'--yes',
'config',
'apply',
'--file',
tomlPath,
]);

const result = docs[0] as { applied: unknown[]; skipped: unknown[] };
expect(result.applied).toHaveLength(1);
expect(result.skipped).toHaveLength(0);

const secretLookups = ossFetchMock.mock.calls.filter(
(c) => c[0] === '/api/secrets/SMTP_PASSWORD',
);
expect(secretLookups).toHaveLength(1);

const putCalls = ossFetchMock.mock.calls.filter(
(c) => c[0] === '/api/auth/smtp-config' && c[1]?.method === 'PUT',
);
expect(putCalls).toHaveLength(1);
const body = JSON.parse(putCalls[0][1]!.body as string) as Record<string, unknown>;
expect(body.password).toBe('real-secret-from-store');
expect(body.host).toBe('smtp.gmail.com');
expect(body.senderEmail).toBe('noreply@app.com');
expect(body.senderName).toBe('App');
expect(body.minIntervalSeconds).toBe(60);

rmSync(tmp, { recursive: true, force: true });
});

it('fails fast (no PUT) when env() ref points at a missing secret', async () => {
nextMetadataResponse = {
auth: { allowedRedirectUrls: [], smtpConfig: EMPTY_SMTP_METADATA },
};

const tomlPath = join(tmp, 'insforge.toml');
writeFileSync(
tomlPath,
`[auth.smtp]
enabled = true
host = "smtp.gmail.com"
port = 587
username = "u@g.com"
password = "env(MISSING_SECRET)"
sender_email = "noreply@app.com"
sender_name = "App"
`,
);

const program = makeProgram();
await expect(
runJson(program, ['--json', '--yes', 'config', 'apply', '--file', tomlPath]),
).rejects.toMatchObject({ code: 'SECRET_NOT_FOUND' });

const putCalls = ossFetchMock.mock.calls.filter(
(c) => c[0] === '/api/auth/smtp-config' && c[1]?.method === 'PUT',
);
expect(putCalls).toHaveLength(0);

rmSync(tmp, { recursive: true, force: true });
});

it("omits password from PUT body when TOML doesn't carry it (default-keep)", async () => {
nextMetadataResponse = {
auth: {
allowedRedirectUrls: [],
smtpConfig: {
...EMPTY_SMTP_METADATA,
enabled: true,
host: 'old.smtp.com',
hasPassword: true,
},
},
};

const tomlPath = join(tmp, 'insforge.toml');
writeFileSync(
tomlPath,
`[auth.smtp]
enabled = true
host = "new.smtp.com"
port = 587
username = ""
sender_email = ""
sender_name = ""
min_interval_seconds = 60
`,
);

const program = makeProgram();
const docs = await runJson(program, [
'--json',
'--yes',
'config',
'apply',
'--file',
tomlPath,
]);

const result = docs[0] as { applied: unknown[]; skipped: unknown[] };
expect(result.applied).toHaveLength(1);

const putCalls = ossFetchMock.mock.calls.filter(
(c) => c[0] === '/api/auth/smtp-config' && c[1]?.method === 'PUT',
);
expect(putCalls).toHaveLength(1);
const body = JSON.parse(putCalls[0][1]!.body as string) as Record<string, unknown>;
expect(body.host).toBe('new.smtp.com');
expect('password' in body).toBe(false);

const secretLookups = ossFetchMock.mock.calls.filter((c) =>
(c[0] as string).startsWith('/api/secrets/'),
);
expect(secretLookups).toHaveLength(0);

rmSync(tmp, { recursive: true, force: true });
});

it('skips SMTP changes when the backend predates the smtpConfig metadata field', async () => {
nextMetadataResponse = { auth: { allowedRedirectUrls: [] } };

const tomlPath = join(tmp, 'insforge.toml');
writeFileSync(
tomlPath,
`[auth.smtp]
enabled = true
host = "smtp.gmail.com"
port = 587
username = "u@g.com"
sender_email = "noreply@app.com"
sender_name = "App"
`,
);

const program = makeProgram();
const docs = await runJson(program, [
'--json',
'--yes',
'config',
'apply',
'--file',
tomlPath,
]);

const result = docs[0] as { applied: unknown[]; skipped: Array<{ key: string }> };
expect(result.applied).toHaveLength(0);
expect(result.skipped).toHaveLength(1);
expect(result.skipped[0].key).toBe('auth.smtp');

const putCalls = ossFetchMock.mock.calls.filter(
(c) => c[0] === '/api/auth/smtp-config' && c[1]?.method === 'PUT',
);
expect(putCalls).toHaveLength(0);

rmSync(tmp, { recursive: true, force: true });
});
});
93 changes: 81 additions & 12 deletions src/commands/config/apply.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,30 @@ import { ossFetch } from '../../lib/api/oss.js';
import { requireAuth } from '../../lib/credentials.js';
import { handleError, getRootOpts, CLIError } from '../../lib/errors.js';
import { parseConfigToml } from '../../lib/config-toml.js';
import { diffConfig, type DiffChange } from '../../lib/config-diff.js';
import { diffConfig, type DiffChange, type LiveConfig } from '../../lib/config-diff.js';
import { formatPlan } from '../../lib/config-format.js';
import { metadataSupports, changePath } from '../../lib/config-capabilities.js';
import type { InsforgeConfig } from '../../lib/config-schema.js';
import { resolveEnvRef } from '../../lib/config-secrets.js';
import { reportCliUsage } from '../../lib/skills.js';

interface RawAuthMetadata {
allowedRedirectUrls?: string[];
smtpConfig?: {
enabled?: boolean;
host?: string;
port?: number;
username?: string;
hasPassword?: boolean;
senderEmail?: string;
senderName?: string;
minIntervalSeconds?: number;
};
}

interface RawMetadataResponse {
auth?: RawAuthMetadata;
}

export function registerConfigApplyCommand(cfg: Command): void {
cfg
.command('apply')
Expand All @@ -31,12 +49,8 @@ export function registerConfigApplyCommand(cfg: Command): void {
const file = parseConfigToml(tomlSource);

const res = await ossFetch('/api/metadata');
const raw = (await res.json()) as {
auth?: { allowedRedirectUrls?: string[] };
};
const live: InsforgeConfig = {
auth: { allowed_redirect_urls: raw.auth?.allowedRedirectUrls ?? [] },
};
const raw = (await res.json()) as RawMetadataResponse;
const live = liveFromMetadata(raw);

const result = diffConfig({ live, file });
const approved = opts.autoApprove || yes;
Expand Down Expand Up @@ -80,9 +94,9 @@ export function registerConfigApplyCommand(cfg: Command): void {
}

// Per-change capability gate. Each change is independent: a backend
// that supports `auth.allowed_redirect_urls` but not (future)
// `email.smtp` should apply the first and skip the second with a
// named warning. Better than failing the whole batch.
// that supports `auth.allowed_redirect_urls` but not `auth.smtp`
// should apply the first and skip the second with a named warning.
// Better than failing the whole batch.
const applied: DiffChange[] = [];
const skipped: Array<{ key: string; reason: string }> = [];
for (const change of result.changes) {
Expand Down Expand Up @@ -126,6 +140,27 @@ export function registerConfigApplyCommand(cfg: Command): void {
});
}

function liveFromMetadata(raw: RawMetadataResponse): LiveConfig {
const live: LiveConfig = { auth: {} };
if (raw.auth?.allowedRedirectUrls !== undefined) {
live.auth!.allowed_redirect_urls = raw.auth.allowedRedirectUrls;
}
if (raw.auth?.smtpConfig) {
const s = raw.auth.smtpConfig;
live.auth!.smtp = {
enabled: s.enabled ?? false,
host: s.host ?? '',
port: s.port ?? 587,
username: s.username ?? '',
hasPassword: s.hasPassword ?? false,
sender_email: s.senderEmail ?? '',
sender_name: s.senderName ?? '',
min_interval_seconds: s.minIntervalSeconds ?? 60,
};
}
return live;
}

async function applyChange(change: DiffChange): Promise<void> {
if (change.section === 'auth' && change.key === 'allowed_redirect_urls') {
await ossFetch('/api/auth/config', {
Expand All @@ -134,5 +169,39 @@ async function applyChange(change: DiffChange): Promise<void> {
});
return;
}
throw new Error(`Unsupported change type: ${change.section}.${change.key}`);
if (change.section === 'auth.smtp') {
// Build the upsert body from the file's resolved view. Force-resend the
// password every time when an env() ref is present — see config-diff.ts
// for the rationale.
const to = change.to;
const body: Record<string, unknown> = {
enabled: to.enabled,
host: to.host,
port: to.port,
username: to.username,
senderEmail: to.sender_email,
senderName: to.sender_name,
minIntervalSeconds: to.min_interval_seconds,
};
if (change.passwordEnvRef) {
// Pre-flight resolves the secret; failure here aborts BEFORE we PUT
// anything, so a missing secret doesn't leave the backend half-updated.
const value = await resolveEnvRef(
`env(${change.passwordEnvRef})`,
'auth.smtp.password',
);
body.password = value;
}
// Omitting `password` from the body tells the backend's upsert to
// preserve the existing encrypted value — matches our "absent = preserve"
// semantics. Force-resend only fires when the TOML carries an env() ref.
await ossFetch('/api/auth/smtp-config', {
method: 'PUT',
body: JSON.stringify(body),
});
return;
}
// Exhaustiveness check — TS will error if we miss a discriminated variant.
const _exhaustive: never = change;
throw new Error(`Unsupported change: ${JSON.stringify(_exhaustive)}`);
}
Loading
Loading