重新整理 cleanup-release 動作與文件 #1
@@ -191,14 +191,52 @@ function requireValue(name, value, displayValue = value) {
|
||||
}
|
||||
|
||||
/**
|
||||
* 驗證字串是否為非負整數。
|
||||
* 驗證字串是否為正整數,且落在安全整數範圍內;下限 1 可避免把保留數設成 0 而清空所有 release。
|
||||
*
|
||||
* @param {string} name 參數名稱。
|
||||
* @param {string} value 參數值。
|
||||
*/
|
||||
function requireInteger(name, value) {
|
||||
if (!/^[0-9]+$/.test(value)) {
|
||||
fail(`${name} must be a non-negative integer`);
|
||||
function requirePositiveInteger(name, value) {
|
||||
if (!/^[0-9]+$/.test(value) || !Number.isSafeInteger(Number(value)) || Number(value) < 1) {
|
||||
fail(`${name} must be a positive integer within the safe integer range`);
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 驗證值是否為合法的 HTTPS 絕對 URL,避免 URL 解析失敗留到主流程中途才拋出。
|
||||
*
|
||||
* @param {string} name 參數名稱。
|
||||
* @param {*} value 參數值。
|
||||
*/
|
||||
|
admin marked this conversation as resolved
Outdated
|
||||
function requireHttpsUrl(name, value) {
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Leo
**問題**:這裡直接 `JSON.parse` 回應內容,沒有包一層具體的錯誤脈絡。只要 API 回傳格式稍微異常,維護者就只會拿到模糊的 syntax error,得重新重現才能知道是哪些 endpoint 出問題。
**建議**:替解析失敗補上更具體的錯誤訊息,至少把 URL 和原始回應片段納入例外,讓除錯時能直接定位是哪一頁資料壞掉。
|
||||
let parsed;
|
||||
try {
|
||||
parsed = new URL(String(value));
|
||||
} catch (error) {
|
||||
fail(`${name} must be a valid absolute URL`);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
if (parsed.protocol !== 'https:') {
|
||||
fail(`${name} must use HTTPS`);
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Leo
**問題**:`processInBatches()` 用 `Promise.all` 搭配外層共享的 `hadFailure`,失敗語意會變得很難推理:一筆例外會直接中斷整批,但其他並行工作仍可能繼續跑,最後到底刪了哪些、漏了哪些,不看執行細節很難判斷。這種控制流對日後補測試或改錯誤處理都不友善。
**建議**:把每筆處理的結果收斂成明確的成功/失敗回傳值,或在批次內逐筆捕捉錯誤後再彙總;若要保留並行,至少讓批次函式回傳可測試的結果集合,而不是依賴外部可變狀態。
|
||||
* 驗證 repository 是否為 `owner/repo` 格式且僅含安全字元,拒絕 `.`、`..` 等路徑片段。
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🔵 建議 **嚴重等級**:🔵 建議
**審查員**:Assassin
**問題**:分頁迴圈沒有上限,只要對方持續回傳非空頁面,這個 action 就會無限抓取;惡意或故障中的 API 可以把 runner 卡死,消耗時間與配額。
**建議**:加入最大頁數、重複頁檢測或總筆數上限,超過就中止並回報異常,避免被外部回應拖成無限迴圈。
|
||||
*
|
||||
* @param {string} name 參數名稱。
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Rogue
**問題**:這裡對每個 request 都先建立 `chunks` 陣列、收完整個 response body,再 `join` 成字串。刪除 release/tag 時大多只需要狀態碼,還硬把回應內容完整緩衝進記憶體,會在大量刪除時增加不必要的配置與拷貝。
**建議**:把 request 包成可選擇是否收集 body;對 DELETE 這類不需要回應內容的呼叫直接丟棄資料串流,只保留 status code。
|
||||
* @param {*} value 參數值。
|
||||
*/
|
||||
function requireRepository(name, value) {
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Assassin
**問題**:這裡把 API 回應 body 直接拼進例外訊息,任何錯誤回應都可能被寫進 action log;如果伺服器或中間層回傳內部路徑、設定值或其他敏感內容,就會被一起外洩。
**建議**:錯誤訊息只保留狀態碼與必要識別資訊,回應內容改成固定摘要或更嚴格的截斷與紅字處理,不要把完整 body 直接丟進例外。
|
||||
const segments = String(value).split('/');
|
||||
const isValidSegment = (segment) =>
|
||||
/^[A-Za-z0-9_.-]+$/.test(segment) && segment !== '.' && segment !== '..';
|
||||
|
||||
if (segments.length !== 2 || !segments.every(isValidSegment)) {
|
||||
fail(`${name} must be in owner/repo format`);
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
@@ -207,10 +245,10 @@ function requireInteger(name, value) {
|
||||
* 對指定 URL 發送 request,回傳狀態碼與 body。
|
||||
*
|
||||
* @param {string} url 完整目標網址。
|
||||
* @param {{ method?: string, headers?: Record<string, string> }} [options] request 設定。
|
||||
* @param {{ method?: string, headers?: Record<string, string>, collectBody?: boolean }} [options] request 設定;`collectBody` 為 `false` 時丟棄回應內容、只保留狀態碼。
|
||||
* @returns {Promise<{ statusCode: number, body: string }>} 回應狀態碼與內容。
|
||||
*/
|
||||
function request(url, { method = 'GET', headers = {} } = {}) {
|
||||
function request(url, { method = 'GET', headers = {}, collectBody = true } = {}) {
|
||||
return new Promise((resolve, reject) => {
|
||||
|
admin marked this conversation as resolved
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Assassin
**問題**:`GITEA_REPOSITORY` 直接字串串進 release API 路徑,沒有做格式驗證或路徑編碼。只要這個值被污染,攻擊者就能把 `../`、額外斜線或其他路徑片段塞進去,讓帶著授權 token 的請求打到非預期的 API 路徑,擴大刪除面。
**建議**:先把 repository 嚴格限制為 `owner/repo` 這種固定格式,再對 owner 與 repo 各自做 `encodeURIComponent` 後組 URL,不要直接把原字串拼進路徑。
|
||||
const target = new URL(url);
|
||||
if (target.protocol !== 'https:') {
|
||||
@@ -226,6 +264,16 @@ function request(url, { method = 'GET', headers = {} } = {}) {
|
||||
agent: keepAliveAgent,
|
||||
},
|
||||
(res) => {
|
||||
const statusCode = res.statusCode || 0;
|
||||
|
||||
if (!collectBody) {
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Maya
**問題**:tag 清理流程新增了『保留已對應 release 的 tag』、『刪除未指定 release 的 tag』、以及無名稱 tag 略過與刪除失敗處理,但目前看不到任何對應測試。這條路徑如果誤刪 tag,會直接破壞版本辨識。
**建議**:補測 tag 清理行為:對應 release 的 tag 必須保留、未被任何 release 引用的 tag 必須被刪除、空名稱 tag 必須略過,並驗證 DELETE 非 204 時會走錯誤分支。
|
||||
res.resume();
|
||||
res.on('end', () => {
|
||||
resolve({ statusCode, body: '' });
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const chunks = [];
|
||||
|
||||
res.setEncoding('utf8');
|
||||
@@ -234,7 +282,7 @@ function request(url, { method = 'GET', headers = {} } = {}) {
|
||||
});
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:`Promise.all` 只要其中一個刪除任務拋出 reject,整個 batch 會立刻失敗,外層 `main().catch(...)` 也會直接結束。這代表只要某一筆 release 或 tag 遇到網路中斷、DNS 失敗、連線逾時,後續同 batch 的項目就不會再處理,清理流程會在半途中停住。
**建議**:把每個 item 的刪除包在個別 `try/catch`,或改用 `Promise.allSettled` 後統一彙總失敗;至少要確保單筆失敗不會中止同批其他清理工作。
|
||||
res.on('end', () => {
|
||||
resolve({
|
||||
statusCode: res.statusCode || 0,
|
||||
statusCode,
|
||||
body: chunks.join(''),
|
||||
});
|
||||
});
|
||||
@@ -300,21 +348,49 @@ async function deleteResource(url, headers) {
|
||||
return request(url, {
|
||||
method: 'DELETE',
|
||||
headers,
|
||||
collectBody: false,
|
||||
});
|
||||
}
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:前面只要有任何 release 刪除失敗,這裡仍然會繼續做 tag 清理,而且 `releaseTags` 是依照刪除前的清單算出來的。最小重現情境是某個待刪 release 因權限不足或暫時性網路錯誤沒刪掉,接著它對應的 tag 仍可能被刪除,最後變成 release 還在、tag 卻被移除的半套狀態。
**建議**:在進入 tag 清理前先檢查 release 刪除是否有失敗;只要有失敗就應中止後續 tag 刪除,或改成只把實際成功刪除的 release 對應 tag 納入待刪集合,避免留下不一致狀態。
|
||||
|
||||
/**
|
||||
* 以固定批次大小處理項目,降低逐筆等待造成的延遲。
|
||||
* 以固定批次大小處理項目,降低逐筆等待造成的延遲;單筆例外不會中止同批其他項目。
|
||||
*
|
||||
* @param {any[]} items 要處理的項目。
|
||||
* @param {number} batchSize 每批同時處理的數量。
|
||||
* @param {(item: any) => Promise<void>} handler 單筆處理函式。
|
||||
* @param {(item: any) => Promise<boolean>} handler 單筆處理函式,回傳該筆是否成功。
|
||||
* @returns {Promise<PromiseSettledResult<boolean>[]>} 依原始順序排列的處理結果。
|
||||
*/
|
||||
async function processInBatches(items, batchSize, handler) {
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Rogue
**問題**:`releaseJson.sort(...)` 先把所有 release 做完整排序,成本是 O(n log n),但後面其實只用前 `KEEP_COUNT` 筆。當 release 很多時,這段排序就是多花 CPU 在不必要的全量比較上。
**建議**:改用固定大小的 top-K 選擇策略,例如維持一個大小為 `KEEP_COUNT` 的最小堆,或在 API 已經有新到舊順序時直接取前 `KEEP_COUNT` 筆,避免整體排序。
|
||||
const results = [];
|
||||
|
||||
for (let index = 0; index < items.length; index += batchSize) {
|
||||
const batch = items.slice(index, index + batchSize);
|
||||
await Promise.all(batch.map((item) => handler(item)));
|
||||
results.push(...(await Promise.allSettled(batch.map((item) => handler(item)))));
|
||||
}
|
||||
|
||||
return results;
|
||||
}
|
||||
|
||||
/**
|
||||
* 彙總批次結果;記錄被 reject 的例外,並回傳是否有任何一筆失敗。
|
||||
*
|
||||
* @param {PromiseSettledResult<boolean>[]} results 批次處理結果。
|
||||
* @returns {boolean} 只要有任一筆失敗即回傳 `true`。
|
||||
*/
|
||||
function hasBatchFailure(results) {
|
||||
let failed = false;
|
||||
|
||||
for (const result of results) {
|
||||
if (result.status === 'rejected') {
|
||||
failed = true;
|
||||
const reason = result.reason;
|
||||
fail(`批次處理發生例外: ${reason instanceof Error ? reason.message : String(reason)}`);
|
||||
} else if (result.value === false) {
|
||||
failed = true;
|
||||
}
|
||||
}
|
||||
|
||||
return failed;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -328,9 +404,11 @@ async function main() {
|
||||
|
||||
section('參數檢查');
|
||||
requireValue('GITEA_SERVER_URL', GITEA_SERVER_URL, maskUrlForLog(GITEA_SERVER_URL));
|
||||
requireHttpsUrl('GITEA_SERVER_URL', GITEA_SERVER_URL);
|
||||
requireValue('GITEA_REPOSITORY', GITEA_REPOSITORY);
|
||||
requireRepository('GITEA_REPOSITORY', GITEA_REPOSITORY);
|
||||
|
admin marked this conversation as resolved
Outdated
admin
commented
嚴重等級:🔵 建議 **嚴重等級**:🔵 建議
**審查員**:Leo
**問題**:在 `catch` 裡先把 `currentStage` 清空再記錄錯誤,會讓最後那筆失敗 log 失去「到底是在哪個階段炸掉」的上下文。等到未來有人要追問題時,只能回頭翻前面的輸出,比對成本會很高。
**建議**:保留最後的 `currentStage`,或在進入 `catch` 時把階段一起寫進錯誤訊息;如果擔心汙染後續輸出,可以在輸出完成後再重設,而不是先清空。
|
||||
requireValue('KEEP_COUNT', KEEP_COUNT);
|
||||
requireInteger('KEEP_COUNT', KEEP_COUNT);
|
||||
requirePositiveInteger('KEEP_COUNT', KEEP_COUNT);
|
||||
|
||||
const keepCount = Number(KEEP_COUNT);
|
||||
const authHeaders = {};
|
||||
@@ -348,7 +426,8 @@ async function main() {
|
||||
serverBase.hash = '';
|
||||
const serverBaseUrl = serverBase.toString().replace(/\/+$/, '');
|
||||
|
||||
const releaseApiUrl = `${serverBaseUrl}/api/v1/repos/${GITEA_REPOSITORY}/releases`;
|
||||
const repositoryPath = GITEA_REPOSITORY.split('/').map(encodeURIComponent).join('/');
|
||||
const releaseApiUrl = `${serverBaseUrl}/api/v1/repos/${repositoryPath}/releases`;
|
||||
|
||||
section('取得成品資訊');
|
||||
info(`GET ${releaseApiUrl}`);
|
||||
@@ -370,21 +449,19 @@ async function main() {
|
||||
info(`RELEASE_COUNT=${releaseCount}`);
|
||||
info(`KEEP_COUNT=${KEEP_COUNT}`);
|
||||
|
||||
let hadFailure = false;
|
||||
|
||||
if (releaseCount <= keepCount) {
|
||||
info('沒有需要清理的舊版本成品');
|
||||
} else {
|
||||
section('刪除舊版本成品');
|
||||
|
||||
const releaseToDelete = releaseJson.slice(keepCount);
|
||||
await processInBatches(releaseToDelete, DELETE_CONCURRENCY, async (releaseItem) => {
|
||||
const releaseResults = await processInBatches(releaseToDelete, DELETE_CONCURRENCY, async (releaseItem) => {
|
||||
const releaseId = releaseItem?.id;
|
||||
if (!Number.isSafeInteger(releaseId) || releaseId <= 0) {
|
||||
|
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:當 `releaseItem.id` 缺失或不是安全整數時,這裡只警告然後回傳 `true`,等於把資料異常當成處理成功。最壞情況是 API 回傳壞資料或 schema 改版,舊 release 被靜默跳過,最後 job 仍可能顯示成功。
**建議**:遇到無效 `id` 時應直接視為失敗,改成 `throw` 或回傳 `false`,讓工作非正常結束並停止後續 tag 清理。
|
||||
warn(
|
||||
`略過 id 不是正整數的成品: ${sanitizeLogText(releaseItem?.tag_name || '')} (${sanitizeLogText(releaseItem?.name || '')})`,
|
||||
);
|
||||
return;
|
||||
return true;
|
||||
}
|
||||
|
||||
const releaseTag = sanitizeLogText(releaseItem.tag_name || '');
|
||||
@@ -395,16 +472,17 @@ async function main() {
|
||||
const { statusCode } = await deleteResource(deleteUrl, authHeaders);
|
||||
if (statusCode === 204) {
|
||||
info(`成功刪除: ${releaseTag} (${releaseName})`);
|
||||
} else {
|
||||
hadFailure = true;
|
||||
fail(`刪除失敗: ${releaseTag} (${releaseName}), HTTP ${statusCode}`);
|
||||
}
|
||||
});
|
||||
return true;
|
||||
}
|
||||
|
||||
if (hadFailure) {
|
||||
fail(`刪除失敗: ${releaseTag} (${releaseName}), HTTP ${statusCode}`);
|
||||
return false;
|
||||
});
|
||||
|
||||
if (hasBatchFailure(releaseResults)) {
|
||||
throw new Error('至少有一筆 release 刪除失敗');
|
||||
}
|
||||
}
|
||||
|
||||
section('刪除未指定 release 的 tag');
|
||||
|
||||
@@ -415,23 +493,23 @@ async function main() {
|
||||
.filter((tag) => !isEmptyOrNull(tag)),
|
||||
);
|
||||
|
||||
const tagApiUrl = `${serverBaseUrl}/api/v1/repos/${GITEA_REPOSITORY}/tags`;
|
||||
const tagApiUrl = `${serverBaseUrl}/api/v1/repos/${repositoryPath}/tags`;
|
||||
info(`GET ${tagApiUrl}`);
|
||||
|
||||
const tagJson = await fetchAllPages(tagApiUrl, authHeaders);
|
||||
info(`TAG_COUNT=${tagJson.length}`);
|
||||
|
||||
await processInBatches(tagJson, DELETE_CONCURRENCY, async (tagItem) => {
|
||||
const tagResults = await processInBatches(tagJson, DELETE_CONCURRENCY, async (tagItem) => {
|
||||
const tagName = tagItem?.name;
|
||||
if (isEmptyOrNull(tagName)) {
|
||||
|
admin
commented
嚴重等級:🟡 警告 **嚴重等級**:🟡 警告
**審查員**:Mage
**問題**:tag 沒有 `name` 時也只是警告後回傳 `true`,這會讓壞資料被靜默略過。若 tag 清單中出現異常項目,cleanup 會看起來成功,但實際上有 tag 沒被處理。
**建議**:把空白或缺失的 `name` 視為失敗,至少讓整體結果反映出資料異常;不要把無法辨識的 tag 當成成功案例。
|
||||
warn('略過沒有名稱的 tag');
|
||||
return;
|
||||
return true;
|
||||
}
|
||||
|
||||
const safeTagName = sanitizeLogText(tagName);
|
||||
if (releaseTags.has(tagName)) {
|
||||
info(`保留指定 release 的 tag: ${safeTagName}`);
|
||||
return;
|
||||
return true;
|
||||
}
|
||||
|
||||
const deleteUrl = `${tagApiUrl}/${encodeURIComponent(tagName)}`;
|
||||
@@ -440,13 +518,14 @@ async function main() {
|
||||
const { statusCode } = await deleteResource(deleteUrl, authHeaders);
|
||||
if (statusCode === 204) {
|
||||
info(`成功刪除未指定 release 的 tag: ${safeTagName}`);
|
||||
} else {
|
||||
hadFailure = true;
|
||||
fail(`刪除 tag 失敗: ${safeTagName}, HTTP ${statusCode}`);
|
||||
return true;
|
||||
}
|
||||
|
||||
fail(`刪除 tag 失敗: ${safeTagName}, HTTP ${statusCode}`);
|
||||
return false;
|
||||
});
|
||||
|
||||
if (hadFailure) {
|
||||
if (hasBatchFailure(tagResults)) {
|
||||
throw new Error('至少有一筆 tag 刪除失敗');
|
||||
}
|
||||
}
|
||||
|
||||
嚴重等級:🟡 警告
審查員:Maya
問題:release 清理流程的排序與切片邏輯現在直接決定會刪掉哪些項目,但沒有測試保證
created_at是由新到舊排序後再依KEEP_COUNT保留。只要排序方向或切片位置錯一格,就會變成刪掉最新的 release。建議:新增 release 清理的整合測試,輸入刻意亂序的
created_at資料,驗證只保留最新KEEP_COUNT筆;再補上KEEP_COUNT=0、KEEP_COUNT=releaseCount與releaseItem.id缺失時會略過刪除的案例。