-
Notifications
You must be signed in to change notification settings - Fork 10
feat(cli): validate sensitive fields require env() references #110
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.
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
Changes from all commits
fc23807
9f8745f
618e90d
a6354a7
296a582
418e398
9bcd5e1
95fe1f1
365f025
8f05013
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| // CLI/src/commands/config/apply.ts | ||
| import type { Command } from 'commander'; | ||
| import { readFileSync } from 'node:fs'; | ||
| import { resolve } from 'node:path'; | ||
| import * as p from '@clack/prompts'; | ||
| import pc from 'picocolors'; | ||
| import { ossFetch } from '../../lib/api/oss.js'; | ||
| import { requireAuth } from '../../lib/credentials.js'; | ||
| import { handleError, getRootOpts } from '../../lib/errors.js'; | ||
| import { parseConfigToml } from '../../lib/config-toml.js'; | ||
| import { diffConfig, type DiffChange } from '../../lib/config-diff.js'; | ||
| import { formatPlan } from '../../lib/config-format.js'; | ||
| import type { InsforgeConfig } from '../../lib/config-schema.js'; | ||
| import { reportCliUsage } from '../../lib/skills.js'; | ||
|
|
||
| export function registerConfigApplyCommand(cfg: Command): void { | ||
| cfg | ||
| .command('apply') | ||
| .description('Apply insforge.toml to the live project') | ||
| .option('--file <path>', 'path to insforge.toml', 'insforge.toml') | ||
| .option('--dry-run', 'show plan, do not apply') | ||
| .option('--auto-approve', 'skip confirmation prompt') | ||
| .action(async (opts, cmd) => { | ||
| const { json } = getRootOpts(cmd); | ||
| try { | ||
| await requireAuth(); | ||
|
|
||
| const tomlPath = resolve(process.cwd(), opts.file); | ||
| const tomlSource = readFileSync(tomlPath, 'utf8'); | ||
| 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 result = diffConfig({ live, file }); | ||
|
|
||
| if (json) { | ||
| console.log(JSON.stringify(result, null, 2)); | ||
| } else { | ||
| console.log(formatPlan(result)); | ||
| } | ||
|
|
||
| if (result.changes.length === 0 || opts.dryRun) { | ||
| await reportCliUsage('cli.config.apply', true); | ||
| return; | ||
| } | ||
|
|
||
| if (!opts.autoApprove && !json) { | ||
| const ok = await p.confirm({ | ||
| message: 'Apply these changes?', | ||
| initialValue: false, | ||
| }); | ||
| if (!ok || p.isCancel(ok)) { | ||
| console.log('Aborted.'); | ||
| await reportCliUsage('cli.config.apply', true); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| for (const change of result.changes) { | ||
| await applyChange(change, file); | ||
| } | ||
|
Comment on lines
+65
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove or underscore the unused Line 87 introduces an unused argument that already shows up in CI lint warnings. Either remove it from Suggested fix- for (const change of result.changes) {
- await applyChange(change, file);
- }
+ for (const change of result.changes) {
+ await applyChange(change);
+ }
@@
-async function applyChange(
- change: DiffChange,
- file: InsforgeConfig,
-): Promise<void> {
+async function applyChange(change: DiffChange): Promise<void> {Also applies to: 85-88 🤖 Prompt for AI Agents |
||
|
|
||
| if (json) { | ||
| console.log(JSON.stringify({ applied: true, changes: result.changes })); | ||
| } else { | ||
| const s = result.summary; | ||
| console.log( | ||
| `${pc.green('✓')} Applied (${s.add} added, ${s.modify} modified, ${s.remove} removed)`, | ||
| ); | ||
| } | ||
| await reportCliUsage('cli.config.apply', true); | ||
| } catch (err) { | ||
| await reportCliUsage('cli.config.apply', false); | ||
| handleError(err, json); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| async function applyChange( | ||
| change: DiffChange, | ||
| file: InsforgeConfig, | ||
| ): Promise<void> { | ||
| if (change.section === 'auth' && change.key === 'allowed_redirect_urls') { | ||
| await ossFetch('/api/auth/config', { | ||
| method: 'PUT', | ||
| body: JSON.stringify({ allowedRedirectUrls: change.to }), | ||
| }); | ||
| return; | ||
| } | ||
| throw new Error(`Unsupported change type: ${change.section}.${change.key}`); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,62 @@ | ||||||||||||||||||||||||
| // CLI/src/commands/config/export.ts | ||||||||||||||||||||||||
| import type { Command } from 'commander'; | ||||||||||||||||||||||||
| import { writeFileSync, existsSync } from 'node:fs'; | ||||||||||||||||||||||||
| import { resolve } from 'node:path'; | ||||||||||||||||||||||||
| import * as p from '@clack/prompts'; | ||||||||||||||||||||||||
| import pc from 'picocolors'; | ||||||||||||||||||||||||
| import { ossFetch } from '../../lib/api/oss.js'; | ||||||||||||||||||||||||
| import { requireAuth } from '../../lib/credentials.js'; | ||||||||||||||||||||||||
| import { handleError, getRootOpts } from '../../lib/errors.js'; | ||||||||||||||||||||||||
| import { stringifyConfigToml } from '../../lib/config-toml.js'; | ||||||||||||||||||||||||
| import type { InsforgeConfig } from '../../lib/config-schema.js'; | ||||||||||||||||||||||||
| import { reportCliUsage } from '../../lib/skills.js'; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export function registerConfigExportCommand(cfg: Command): void { | ||||||||||||||||||||||||
| cfg | ||||||||||||||||||||||||
| .command('export') | ||||||||||||||||||||||||
| .description('Pull live project config and write insforge.toml') | ||||||||||||||||||||||||
| .option('--out <path>', 'output path', 'insforge.toml') | ||||||||||||||||||||||||
| .option('--force', 'overwrite without confirmation') | ||||||||||||||||||||||||
| .action(async (opts, cmd) => { | ||||||||||||||||||||||||
| const { json } = getRootOpts(cmd); | ||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||
| await requireAuth(); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const target = resolve(process.cwd(), opts.out); | ||||||||||||||||||||||||
| if (existsSync(target) && !opts.force) { | ||||||||||||||||||||||||
| const ok = await p.confirm({ | ||||||||||||||||||||||||
| message: `${opts.out} exists. Overwrite?`, | ||||||||||||||||||||||||
| initialValue: false, | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| if (!ok || p.isCancel(ok)) { | ||||||||||||||||||||||||
| console.log('Aborted.'); | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const res = await ossFetch('/api/metadata'); | ||||||||||||||||||||||||
| const raw = (await res.json()) as { | ||||||||||||||||||||||||
| auth?: { allowedRedirectUrls?: string[] }; | ||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const config: InsforgeConfig = { | ||||||||||||||||||||||||
| auth: { | ||||||||||||||||||||||||
| allowed_redirect_urls: raw.auth?.allowedRedirectUrls ?? [], | ||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
|
Comment on lines
+42
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don’t synthesize Suggested patch- const config: InsforgeConfig = {
- auth: {
- allowed_redirect_urls: raw.auth?.allowedRedirectUrls ?? [],
- },
- };
+ const redirects = raw.auth?.allowedRedirectUrls;
+ const config: InsforgeConfig =
+ redirects === undefined
+ ? {}
+ : {
+ auth: { allowed_redirect_urls: redirects },
+ };📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const toml = stringifyConfigToml(config); | ||||||||||||||||||||||||
| writeFileSync(target, toml, 'utf8'); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if (json) { | ||||||||||||||||||||||||
| console.log(JSON.stringify({ written: target, config })); | ||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||
| console.log(`${pc.green('✓')} Wrote ${target}`); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| await reportCliUsage('cli.config.export', true); | ||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||
| await reportCliUsage('cli.config.export', false); | ||||||||||||||||||||||||
| handleError(err, json); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| // CLI/src/commands/config/index.ts | ||
| import type { Command } from 'commander'; | ||
| import { registerConfigExportCommand } from './export.js'; | ||
| import { registerConfigPlanCommand } from './plan.js'; | ||
| import { registerConfigApplyCommand } from './apply.js'; | ||
|
|
||
| export function registerConfigCommand(program: Command): void { | ||
| const cfg = program | ||
| .command('config') | ||
| .description('Manage insforge.toml (declarative project configuration)'); | ||
| registerConfigExportCommand(cfg); | ||
| registerConfigPlanCommand(cfg); | ||
| registerConfigApplyCommand(cfg); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| // CLI/src/commands/config/plan.ts | ||
| import type { Command } from 'commander'; | ||
| import { readFileSync } from 'node:fs'; | ||
| import { resolve } from 'node:path'; | ||
| import { ossFetch } from '../../lib/api/oss.js'; | ||
| import { requireAuth } from '../../lib/credentials.js'; | ||
| import { handleError, getRootOpts } from '../../lib/errors.js'; | ||
| import { parseConfigToml } from '../../lib/config-toml.js'; | ||
| import { diffConfig } from '../../lib/config-diff.js'; | ||
| import { formatPlan } from '../../lib/config-format.js'; | ||
| import type { InsforgeConfig } from '../../lib/config-schema.js'; | ||
| import { reportCliUsage } from '../../lib/skills.js'; | ||
|
|
||
| export function registerConfigPlanCommand(cfg: Command): void { | ||
| cfg | ||
| .command('plan') | ||
| .description('Show diff between insforge.toml and live project state') | ||
| .option('--file <path>', 'path to insforge.toml', 'insforge.toml') | ||
| .action(async (opts, cmd) => { | ||
| const { json } = getRootOpts(cmd); | ||
| try { | ||
| await requireAuth(); | ||
|
|
||
| const tomlPath = resolve(process.cwd(), opts.file); | ||
| const tomlSource = readFileSync(tomlPath, 'utf8'); | ||
| 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 result = diffConfig({ live, file }); | ||
|
|
||
| if (json) { | ||
| console.log(JSON.stringify(result, null, 2)); | ||
| } else { | ||
| console.log(`Plan for insforge.toml (file: ${opts.file}):\n`); | ||
| console.log(formatPlan(result)); | ||
| } | ||
| await reportCliUsage('cli.config.plan', true); | ||
| } catch (err) { | ||
| await reportCliUsage('cli.config.plan', false); | ||
| handleError(err, json); | ||
| } | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { diffConfig } from './config-diff.js'; | ||
|
|
||
| describe('diffConfig', () => { | ||
| it('detects an array change in allowed_redirect_urls', () => { | ||
| const live = { auth: { allowed_redirect_urls: ['https://a.com'] } }; | ||
| const file = { auth: { allowed_redirect_urls: ['https://a.com', 'https://b.com'] } }; | ||
| expect(diffConfig({ live, file })).toEqual({ | ||
| changes: [ | ||
| { | ||
| section: 'auth', | ||
| op: 'modify', | ||
| key: 'allowed_redirect_urls', | ||
| from: ['https://a.com'], | ||
| to: ['https://a.com', 'https://b.com'], | ||
| }, | ||
| ], | ||
| summary: { add: 0, modify: 1, remove: 0, kept: 0 }, | ||
| }); | ||
| }); | ||
|
|
||
| it('returns no changes for converged state', () => { | ||
| const same = { auth: { allowed_redirect_urls: ['https://a.com'] } }; | ||
| expect(diffConfig({ live: same, file: same })).toEqual({ | ||
| changes: [], | ||
| summary: { add: 0, modify: 0, remove: 0, kept: 0 }, | ||
| }); | ||
| }); | ||
|
|
||
| it('treats missing field in file as no-op (no remove)', () => { | ||
| const live = { auth: { allowed_redirect_urls: ['https://a.com'] } }; | ||
| const file = {}; | ||
| expect(diffConfig({ live, file })).toEqual({ | ||
| changes: [], | ||
| summary: { add: 0, modify: 0, remove: 0, kept: 0 }, | ||
| }); | ||
| }); | ||
|
|
||
| it('treats empty-array vs non-empty as a real change', () => { | ||
| const live = { auth: { allowed_redirect_urls: ['https://a.com'] } }; | ||
| const file = { auth: { allowed_redirect_urls: [] } }; | ||
| expect(diffConfig({ live, file }).changes).toEqual([ | ||
| { | ||
| section: 'auth', | ||
| op: 'modify', | ||
| key: 'allowed_redirect_urls', | ||
| from: ['https://a.com'], | ||
| to: [], | ||
| }, | ||
| ]); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--jsoncurrently emits two separate JSON documents.At Line 43 and Line 70, the command prints two top-level JSON payloads in a single run when changes are applied. That breaks parsers expecting one JSON document per invocation.
Suggested fix
Also applies to: 69-71
🤖 Prompt for AI Agents