fix: validate release asset uploads
All checks were successful
labels / labels (pull_request) Successful in 8s
ci / test (pull_request) Successful in 14s

This commit is contained in:
codex-bot-andresmgsl 2026-08-30 11:08:01 +00:00
parent 8293c83531
commit 3c0709189e
4 changed files with 36 additions and 3 deletions

View file

@ -18,6 +18,8 @@ const REQUEST_TIMEOUT_MS = 30000;
// Repository migrations clone the full source repository and can legitimately // Repository migrations clone the full source repository and can legitimately
// take minutes, so they get a much longer budget. // take minutes, so they get a much longer budget.
const MIGRATE_TIMEOUT_MS = 10 * 60 * 1000; const MIGRATE_TIMEOUT_MS = 10 * 60 * 1000;
// Release assets can be much larger than JSON API payloads, so uploads get a
// separate budget while retaining the standard timeout for ordinary calls.
const UPLOAD_TIMEOUT_MS = 10 * 60 * 1000; const UPLOAD_TIMEOUT_MS = 10 * 60 * 1000;
class ForgejoClient { class ForgejoClient {
@ -274,7 +276,12 @@ class ForgejoClient {
async uploadReleaseAsset(owner, repo, releaseId, filePath, name) { async uploadReleaseAsset(owner, repo, releaseId, filePath, name) {
const form = new FormData(); const form = new FormData();
const file = await fs.openAsBlob(filePath); let file;
try {
file = await fs.openAsBlob(filePath);
} catch (err) {
throw new Error(`Could not read asset file ${filePath}: ${err.message}`);
}
form.append('attachment', file, name); form.append('attachment', file, name);
const query = new URLSearchParams({ name }); const query = new URLSearchParams({ name });
return this.uploadRequest( return this.uploadRequest(

View file

@ -97,7 +97,10 @@ function collectOption(value, previous) {
return previous.concat(value); return previous.concat(value);
} }
function validateAssetOptions(options) { function validateAssetOptions(options, { requireAsset = false } = {}) {
if (requireAsset && options.asset.length === 0) {
throw new Error('At least one --asset is required.');
}
if (options.assetName && options.asset.length !== 1) { if (options.assetName && options.asset.length !== 1) {
throw new Error('--asset-name requires exactly one --asset.'); throw new Error('--asset-name requires exactly one --asset.');
} }
@ -1150,7 +1153,7 @@ release
.option('--asset-name <name>', 'override the uploaded filename (exactly one asset)') .option('--asset-name <name>', 'override the uploaded filename (exactly one asset)')
.action(async (options) => { .action(async (options) => {
try { try {
validateAssetOptions(options); validateAssetOptions(options, { requireAsset: true });
const config = loadConfig(); const config = loadConfig();
const client = ForgejoClient.fromConfig(config); const client = ForgejoClient.fromConfig(config);
const releaseResult = await client.getReleaseByTag(options.owner, options.repo, options.tag); const releaseResult = await client.getReleaseByTag(options.owner, options.repo, options.tag);

View file

@ -305,6 +305,20 @@ test('uploadReleaseAsset streams multipart data without forcing a JSON content t
} }
}); });
test('uploadReleaseAsset identifies a missing local asset path', async () => {
const assetPath = path.join(os.tmpdir(), `stoke-missing-asset-${process.pid}.bin`);
const client = new ForgejoClient('https://forge.test', 'tok');
await assert.rejects(
client.uploadReleaseAsset('owner', 'repo', 42, assetPath, 'artifact.bin'),
(err) => {
assert.match(err.message, /Could not read asset file/);
assert.match(err.message, new RegExp(assetPath.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')));
return true;
},
);
});
test('uploadReleaseAsset uses the upload timeout instead of the 30 second JSON timeout', async () => { test('uploadReleaseAsset uses the upload timeout instead of the 30 second JSON timeout', async () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'stoke-upload-timeout-')); const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'stoke-upload-timeout-'));
const assetPath = path.join(dir, 'large.bin'); const assetPath = path.join(dir, 'large.bin');

View file

@ -85,6 +85,15 @@ test('release create rejects one asset name for multiple assets before reading c
assert.doesNotMatch(res.stderr, /Not authenticated/); assert.doesNotMatch(res.stderr, /Not authenticated/);
}); });
test('release upload rejects zero assets before reading config', () => {
const res = run([
'release', 'upload', '-o', 'o', '-r', 'r', '--tag', 'v1',
], { STOKE_CONFIG_FILE: path.join(os.tmpdir(), `stoke-none-${process.pid}-release.json`) });
assert.equal(res.status, 1);
assert.match(res.stderr, /at least one --asset is required/i);
assert.doesNotMatch(res.stderr, /Not authenticated/);
});
test('release create prints the id and uploads every asset as multipart data', async () => { test('release create prints the id and uploads every asset as multipart data', async () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'stoke-release-create-')); const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'stoke-release-create-'));
const cfg = path.join(dir, 'config.json'); const cfg = path.join(dir, 'config.json');