Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
12 changes: 12 additions & 0 deletions src/commands/compute/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { requireAuth } from '../../lib/credentials.js';
import { handleError, getRootOpts, CLIError } from '../../lib/errors.js';
import { outputJson, outputSuccess, outputInfo } from '../../lib/output.js';
import { reportCliUsage } from '../../lib/skills.js';
import { parseEnvFile } from '../../lib/env-file.js';
import {
ensureDockerAvailable,
dockerBuild,
Expand Down Expand Up @@ -42,6 +43,10 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
.option('--memory <mb>', 'Memory in MB', '512')
.option('--region <region>', 'Fly.io region', 'iad')
.option('--env <json>', 'Env vars as JSON object')
.option(
'--env-file <path>',
'Path to a .env file (KEY=VALUE per line, #-comments + blank lines ok). Mutually exclusive with --env.'
)
.action(async (dir: string | undefined, opts, cmd) => {
const { json } = getRootOpts(cmd);
try {
Expand All @@ -65,6 +70,11 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
if (!Number.isInteger(memory) || memory <= 0) {
throw new CLIError(`Invalid --memory: ${opts.memory}`);
}
if (opts.env && opts.envFile) {
throw new CLIError(
'--env and --env-file are mutually exclusive — pick one source for the env vars.'
);
}
let envVars: Record<string, string> | undefined;
if (opts.env) {
let parsed: unknown;
Expand All @@ -84,6 +94,8 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
}
}
envVars = parsed as Record<string, string>;
} else if (opts.envFile) {
envVars = parseEnvFile(resolve(opts.envFile));
}

const baseBody: Record<string, unknown> = {
Expand Down
76 changes: 74 additions & 2 deletions src/commands/compute/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,38 @@ import { handleError, getRootOpts, CLIError } from '../../lib/errors.js';
import { outputJson, outputSuccess } from '../../lib/output.js';
import { reportCliUsage } from '../../lib/skills.js';

const ENV_KEY_REGEX = /^[A-Z_][A-Z0-9_]*$/;

// Commander collector for repeatable flags. Each invocation appends to the
// running list rather than overwriting.
function collect(value: string, previous: string[]): string[] {
return previous.concat([value]);
}

// Parse a "KEY=VALUE" string into a tuple, validating the key shape against
// the same regex the OSS schema enforces. Values may contain '=' and are
// preserved verbatim — only the first '=' separates key from value.
function parseKeyValue(raw: string): [string, string] {
const eq = raw.indexOf('=');
if (eq <= 0) {
throw new CLIError(
`Invalid --env-set "${raw}": expected KEY=VALUE (key first, then '=', then value)`
);
}
const key = raw.slice(0, eq);
const value = raw.slice(eq + 1);
if (!ENV_KEY_REGEX.test(key)) {
throw new CLIError(`Invalid env var key "${key}": must match [A-Z_][A-Z0-9_]*`);
}
return [key, value];
}

function assertValidKey(key: string): void {
if (!ENV_KEY_REGEX.test(key)) {
throw new CLIError(`Invalid env var key "${key}": must match [A-Z_][A-Z0-9_]*`);
}
}

export function registerComputeUpdateCommand(computeCmd: Command): void {
computeCmd
.command('update <id>')
Expand All @@ -14,7 +46,22 @@ export function registerComputeUpdateCommand(computeCmd: Command): void {
.option('--cpu <tier>', 'CPU tier')
.option('--memory <mb>', 'Memory in MB')
.option('--region <region>', 'Fly.io region')
.option('--env <json>', 'Environment variables as JSON object')
.option(
'--env <json>',
'Replace ALL env vars with this JSON object. To rotate one secret without restating the others, use --env-set instead.'
)
.option(
'--env-set <KEY=VALUE>',
'Set or update one env var (repeatable). Merges with existing — does not clear other vars.',
collect,
[]
)
.option(
'--env-unset <KEY>',
'Remove one env var (repeatable). Merges with existing — leaves other vars in place.',
collect,
[]
)
.action(async (id: string, opts, cmd) => {
const { json } = getRootOpts(cmd);
try {
Expand All @@ -37,6 +84,16 @@ export function registerComputeUpdateCommand(computeCmd: Command): void {
}
if (opts.region) body.region = opts.region;

const envSetArgs = opts.envSet as string[];
const envUnsetArgs = opts.envUnset as string[];
const hasPatch = envSetArgs.length > 0 || envUnsetArgs.length > 0;

if (opts.env && hasPatch) {
throw new CLIError(
'--env (wholesale replace) and --env-set/--env-unset (partial merge) are mutually exclusive — pick one.'
);
}

if (opts.env) {
try {
body.envVars = JSON.parse(opts.env);
Expand All @@ -45,8 +102,23 @@ export function registerComputeUpdateCommand(computeCmd: Command): void {
}
}

if (hasPatch) {
const setMap: Record<string, string> = {};
for (const arg of envSetArgs) {
const [k, v] = parseKeyValue(arg);
setMap[k] = v;
}
for (const k of envUnsetArgs) assertValidKey(k);
body.envVarsPatch = {
...(envSetArgs.length > 0 && { set: setMap }),
...(envUnsetArgs.length > 0 && { unset: envUnsetArgs }),
};
Comment on lines +105 to +115
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 | 🟠 Major | ⚡ Quick win

Reject conflicting env patch operations for the same key.

A key can currently be present in both set and unset, creating an ambiguous patch contract. Fail fast in CLI when overlap exists.

Suggested fix
         if (hasPatch) {
           const setMap: Record<string, string> = {};
           for (const arg of envSetArgs) {
             const [k, v] = parseKeyValue(arg);
             setMap[k] = v;
           }
           for (const k of envUnsetArgs) assertValidKey(k);
+          const overlaps = envUnsetArgs.filter((k) => Object.hasOwn(setMap, k));
+          if (overlaps.length > 0) {
+            throw new CLIError(
+              `Conflicting env patch: key(s) present in both --env-set and --env-unset: ${overlaps.join(', ')}`
+            );
+          }
           body.envVarsPatch = {
             ...(envSetArgs.length > 0 && { set: setMap }),
             ...(envUnsetArgs.length > 0 && { unset: envUnsetArgs }),
           };
         }
📝 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
if (hasPatch) {
const setMap: Record<string, string> = {};
for (const arg of envSetArgs) {
const [k, v] = parseKeyValue(arg);
setMap[k] = v;
}
for (const k of envUnsetArgs) assertValidKey(k);
body.envVarsPatch = {
...(envSetArgs.length > 0 && { set: setMap }),
...(envUnsetArgs.length > 0 && { unset: envUnsetArgs }),
};
if (hasPatch) {
const setMap: Record<string, string> = {};
for (const arg of envSetArgs) {
const [k, v] = parseKeyValue(arg);
setMap[k] = v;
}
for (const k of envUnsetArgs) assertValidKey(k);
const overlaps = envUnsetArgs.filter((k) => Object.hasOwn(setMap, k));
if (overlaps.length > 0) {
throw new CLIError(
`Conflicting env patch: key(s) present in both --env-set and --env-unset: ${overlaps.join(', ')}`
);
}
body.envVarsPatch = {
...(envSetArgs.length > 0 && { set: setMap }),
...(envUnsetArgs.length > 0 && { unset: envUnsetArgs }),
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/compute/update.ts` around lines 105 - 115, The env patch
building in update.ts allows the same key to appear in both envSetArgs and
envUnsetArgs, causing ambiguous patches; before constructing setMap and
assigning body.envVarsPatch (in the block guarded by hasPatch), validate and
reject overlaps by computing the intersection of keys produced by envSetArgs
(use parseKeyValue to extract keys) and envUnsetArgs (after assertValidKey), and
if any duplicates exist throw a user-facing error (e.g., exit or throw)
explaining the conflicting keys so the CLI fails fast rather than producing a
patch where a key is both set and unset.

}

if (Object.keys(body).length === 0) {
throw new CLIError('No update fields provided. Use --image, --port, --cpu, --memory, --region, or --env.');
throw new CLIError(
'No update fields provided. Use --image, --port, --cpu, --memory, --region, --env, --env-set, or --env-unset.'
);
}

const res = await ossFetch(`/api/compute/services/${encodeURIComponent(id)}`, {
Expand Down
5 changes: 4 additions & 1 deletion src/commands/schedules/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ export function registerSchedulesCreateCommand(schedulesCmd: Command): void {
.command('create')
.description('Create a new schedule')
.requiredOption('--name <name>', 'Schedule name')
.requiredOption('--cron <expression>', 'Cron expression (5-field format)')
.requiredOption(
'--cron <expression>',
'Cron expression. 5-field cron (e.g. "*/5 * * * *", "0 9 * * 1-5") or pg_cron interval syntax for sub-minute cadence (e.g. "30 seconds", "5 minutes").'
)
.requiredOption('--url <url>', 'URL to invoke')
.requiredOption('--method <method>', 'HTTP method (GET, POST, PUT, PATCH, DELETE)')
.option('--headers <json>', 'HTTP headers as JSON')
Expand Down
5 changes: 4 additions & 1 deletion src/commands/schedules/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ export function registerSchedulesUpdateCommand(schedulesCmd: Command): void {
.command('update <id>')
.description('Update a schedule')
.option('--name <name>', 'New schedule name')
.option('--cron <expression>', 'New cron expression')
.option(
'--cron <expression>',
'New cron expression. 5-field cron or pg_cron interval syntax (e.g. "30 seconds").'
)
.option('--url <url>', 'New URL to invoke')
.option('--method <method>', 'New HTTP method')
.option('--headers <json>', 'New HTTP headers as JSON')
Expand Down
82 changes: 82 additions & 0 deletions src/lib/env-file.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { describe, expect, it, beforeAll, afterAll } from 'vitest';
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { parseEnvFile } from './env-file.js';

let dir: string;

beforeAll(() => {
dir = mkdtempSync(join(tmpdir(), 'cli-env-file-'));
});

afterAll(() => {
rmSync(dir, { recursive: true, force: true });
});

function write(name: string, contents: string): string {
const p = join(dir, name);
writeFileSync(p, contents);
return p;
}

describe('parseEnvFile', () => {
it('parses plain KEY=VALUE pairs', () => {
const p = write('plain.env', 'FOO=bar\nBAZ=qux\n');
expect(parseEnvFile(p)).toEqual({ FOO: 'bar', BAZ: 'qux' });
});

it('skips blank lines and # comments', () => {
const p = write('comments.env', '# header comment\n\nFOO=bar\n# inline\nBAZ=qux\n');
expect(parseEnvFile(p)).toEqual({ FOO: 'bar', BAZ: 'qux' });
});

it('strips matching surrounding double quotes from values', () => {
const p = write('dquotes.env', 'GREETING="hello world"\n');
expect(parseEnvFile(p)).toEqual({ GREETING: 'hello world' });
});

it('strips matching surrounding single quotes from values', () => {
const p = write('squotes.env', "GREETING='hello world'\n");
expect(parseEnvFile(p)).toEqual({ GREETING: 'hello world' });
});

it('preserves # inside quoted values (not a comment)', () => {
const p = write('hash.env', 'PASSWORD="abc#123"\n');
expect(parseEnvFile(p)).toEqual({ PASSWORD: 'abc#123' });
});

it('strips trailing inline comment from unquoted values', () => {
const p = write('inline.env', 'PORT=8080 # default port\n');
expect(parseEnvFile(p)).toEqual({ PORT: '8080' });
});

it('preserves "=" inside values (only first equals splits)', () => {
const p = write('eq.env', 'JWT=a.b=c.d\n');
expect(parseEnvFile(p)).toEqual({ JWT: 'a.b=c.d' });
});

it('rejects invalid keys (lowercase, hyphen)', () => {
const p = write('badkey.env', 'lower=ok\n');
expect(() => parseEnvFile(p)).toThrow(/invalid env var key/);
});

it('rejects malformed lines (no equals)', () => {
const p = write('malformed.env', 'NOT_A_PAIR\n');
expect(() => parseEnvFile(p)).toThrow(/expected KEY=VALUE/);
});

it('reports the line number on errors', () => {
const p = write('linenum.env', 'GOOD=ok\n\nBAD\n');
expect(() => parseEnvFile(p)).toThrow(/:3:/);
});

it('throws CLIError when file does not exist', () => {
expect(() => parseEnvFile(join(dir, 'nope.env'))).toThrow(/Could not read --env-file/);
});

it('handles CRLF line endings', () => {
const p = write('crlf.env', 'FOO=bar\r\nBAZ=qux\r\n');
expect(parseEnvFile(p)).toEqual({ FOO: 'bar', BAZ: 'qux' });
});
});
62 changes: 62 additions & 0 deletions src/lib/env-file.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { readFileSync } from 'node:fs';
import { CLIError } from './errors.js';

const ENV_KEY_REGEX = /^[A-Z_][A-Z0-9_]*$/;

// Minimal dotenv parser. Handles:
// • KEY=VALUE lines
// • Blank lines and # comment lines
// • Optional surrounding quotes ("..." or '...') stripped from VALUE
// • Inline trailing comments after unquoted values: KEY=val # note
//
// Keeps escape-sequence handling deliberately out of scope — anything fancy
// (multiline strings, $VAR expansion) belongs in a real dotenv library; for
// `compute deploy --env-file` the goal is feature-parity with `--env <json>`
// for the 95% case, not full dotenv semantics.
export function parseEnvFile(path: string): Record<string, string> {
let raw: string;
try {
raw = readFileSync(path, 'utf-8');
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
throw new CLIError(`Could not read --env-file at ${path}: ${msg}`);
}

const result: Record<string, string> = {};
const lines = raw.split(/\r?\n/);
for (let i = 0; i < lines.length; i++) {
const line = lines[i].trim();
if (line === '' || line.startsWith('#')) continue;

const eq = line.indexOf('=');
if (eq <= 0) {
throw new CLIError(
`${path}:${i + 1}: expected KEY=VALUE, got "${line}"`
);
}
const key = line.slice(0, eq).trim();
if (!ENV_KEY_REGEX.test(key)) {
throw new CLIError(
`${path}:${i + 1}: invalid env var key "${key}" (must match [A-Z_][A-Z0-9_]*)`
);
}

let value = line.slice(eq + 1).trim();

// Surrounding quotes (matching pair) — strip them and use the inner
// string verbatim. Anything inside quotes is preserved including '#'.
if (
(value.startsWith('"') && value.endsWith('"') && value.length >= 2) ||
(value.startsWith("'") && value.endsWith("'") && value.length >= 2)
) {
value = value.slice(1, -1);
} else {
// Unquoted value — strip a trailing inline comment (`KEY=val # note`).
const hash = value.indexOf(' #');
if (hash >= 0) value = value.slice(0, hash).trimEnd();
}

result[key] = value;
}
return result;
}
Loading