fix: validate release asset uploads
This commit is contained in:
parent
8293c83531
commit
3c0709189e
4 changed files with 36 additions and 3 deletions
|
|
@ -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(
|
||||||
|
|
|
||||||
|
|
@ -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);
|
||||||
|
|
|
||||||
|
|
@ -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');
|
||||||
|
|
|
||||||
|
|
@ -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');
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue