Add zip-extraction dependency (adm-zip vs yauzl) #112

New issue

Closed

opened 2026-08-29 03:27:51 +00:00 by david · 1 comment

david commented

2026-08-29 03:27:51 +00:00

Owner

Summary

Add the zip-extraction dependency the run-logs download step needs. Node has no built-in unzip, so downloadRunLogs must use a library to extract the Forgejo Actions run-logs ZIP (GET /repos/{owner}/{repo}/actions/runs/{run_id}/logs).

Background

Part of the Forgejo Actions tooling feature (tracking issue #111 — work on branch feature/issue-111/forgejo-actions-tools, commits prefixed issue-111: ). This step only adds the dependency and documents the choice; the extraction code itself lands in the downloadRunLogs step. The run-logs endpoint returns a ZIP archive of all jobs' logs, so the chosen library must read a ZIP from a Buffer (the download is a single fetch) and extract individual entries with size controls.

Two candidates, per the plan:

adm-zip — sync, simple, 0 dependencies (@types/adm-zip).
yauzl — async/streaming, memory-bounded, built-in filename validation + validateEntrySizes zip-bomb defense (@types/yauzl).

Documentation Required

A separate process downloads these into docs/reference/adm-zip/ and docs/reference/yauzl/ before this issue is implemented. Check those folders for the actual reference material before starting.

docs/reference/adm-zip/
- https://www.npmjs.com/package/adm-zip — npm page for adm-zip 0.6.0: install, sync API overview (getEntries, readAsText, extractAllTo, extractEntryTo, toBuffer), 0 dependencies, MIT.
- https://github.com/cthackers/adm-zip — GitHub repo; links to the wiki with detailed API (constructor options incl. fs override, entry iteration, extraction, in-memory buffers).
- https://github.com/cthackers/adm-zip/wiki — detailed API documentation (entries, extraction modes, buffering).
docs/reference/yauzl/
- https://www.npmjs.com/package/yauzl — npm page for yauzl 3.4.0: full API reference — open/fromBuffer/openPromise, eachEntry() async iterator, openReadStream, validateEntrySizes, validateFileName; design principles (async, memory-bounded, no streaming-unzip API).
- https://github.com/thejoshwolfe/yauzl — GitHub repo; examples/ for promise and callback usage; notes on zip-bomb heuristics and spec conformance.

Implementation Details

Decide the winner using these criteria (document the decision in a comment on this issue):
- Zip-bomb safety: yauzl enforces reported-vs-actual uncompressed sizes (validateEntrySizes) and validates entry filenames (validateFileName); adm-zip needs manual size checks on getData().
- API style: adm-zip is synchronous (simple, blocking); yauzl is async streaming (non-blocking, better for large logs).
- Types: both have @types packages; verify they are current.
Add the chosen library to extensions/forgejo/package.json dependencies (it ships with the extension — not a devDependency), plus the matching @types/* in devDependencies.
npm install in extensions/forgejo/ and confirm package-lock.json updates.
Keep npm run check (tsc --noEmit) green.

Recommended: yauzl if zip-bomb resistance and memory-boundedness are prioritized (the run-logs ZIP comes from a remote host and can be large); adm-zip if sync simplicity wins. Either is acceptable — the plan explicitly leaves the choice open.

Acceptance Criteria

Exactly one of adm-zip or yauzl is a runtime dependency of extensions/forgejo/ (plus its @types as devDependency).
npm install completed; package.json + package-lock.json updated; no extraneous deps.
npm run check (tsc --noEmit) passes in extensions/forgejo/.
The chosen library's API (entry iteration + per-entry extraction from a Buffer) is verified with a tiny throwaway script or a jest test reading a small fixture ZIP.
The choice and rationale are documented (comment on this issue or a note in src/actions.ts).

Test Plan

Run cd extensions/forgejo && npm install && npm run check.
Verify the dependency resolves: node -e "require('yauzl')" (or adm-zip) prints no error.
Create a 2-entry ZIP (zip CLI or the library itself) and confirm entries can be listed and extracted to an os.tmpdir() subdir.

### Summary Add the zip-extraction dependency the run-logs download step needs. Node has **no built-in unzip**, so `downloadRunLogs` must use a library to extract the Forgejo Actions run-logs ZIP (`GET /repos/{owner}/{repo}/actions/runs/{run_id}/logs`). ### Background Part of the Forgejo Actions tooling feature (tracking issue **#111** — work on branch `feature/issue-111/forgejo-actions-tools`, commits prefixed `issue-111: `). This step only adds the dependency and documents the choice; the extraction code itself lands in the `downloadRunLogs` step. The run-logs endpoint returns a ZIP archive of all jobs' logs, so the chosen library must read a ZIP from a Buffer (the download is a single `fetch`) and extract individual entries with size controls. Two candidates, per the plan: - **adm-zip** — sync, simple, 0 dependencies (`@types/adm-zip`). - **yauzl** — async/streaming, memory-bounded, built-in filename validation + `validateEntrySizes` zip-bomb defense (`@types/yauzl`). ### Documentation Required A separate process downloads these into `docs/reference/adm-zip/` and `docs/reference/yauzl/` before this issue is implemented. **Check those folders for the actual reference material before starting.** - `docs/reference/adm-zip/` - https://www.npmjs.com/package/adm-zip — npm page for adm-zip 0.6.0: install, sync API overview (`getEntries`, `readAsText`, `extractAllTo`, `extractEntryTo`, `toBuffer`), 0 dependencies, MIT. - https://github.com/cthackers/adm-zip — GitHub repo; links to the wiki with detailed API (constructor options incl. `fs` override, entry iteration, extraction, in-memory buffers). - https://github.com/cthackers/adm-zip/wiki — detailed API documentation (entries, extraction modes, buffering). - `docs/reference/yauzl/` - https://www.npmjs.com/package/yauzl — npm page for yauzl 3.4.0: full API reference — `open`/`fromBuffer`/`openPromise`, `eachEntry()` async iterator, `openReadStream`, `validateEntrySizes`, `validateFileName`; design principles (async, memory-bounded, no streaming-unzip API). - https://github.com/thejoshwolfe/yauzl — GitHub repo; `examples/` for promise and callback usage; notes on zip-bomb heuristics and spec conformance. ### Implementation Details 1. Decide the winner using these criteria (document the decision in a comment on this issue): - **Zip-bomb safety**: yauzl enforces reported-vs-actual uncompressed sizes (`validateEntrySizes`) and validates entry filenames (`validateFileName`); adm-zip needs manual size checks on `getData()`. - **API style**: adm-zip is synchronous (simple, blocking); yauzl is async streaming (non-blocking, better for large logs). - **Types**: both have `@types` packages; verify they are current. 2. Add the chosen library to `extensions/forgejo/package.json` **`dependencies`** (it ships with the extension — not a devDependency), plus the matching `@types/*` in `devDependencies`. 3. `npm install` in `extensions/forgejo/` and confirm `package-lock.json` updates. 4. Keep `npm run check` (tsc --noEmit) green. Recommended: **yauzl** if zip-bomb resistance and memory-boundedness are prioritized (the run-logs ZIP comes from a remote host and can be large); **adm-zip** if sync simplicity wins. Either is acceptable — the plan explicitly leaves the choice open. ### Acceptance Criteria - [ ] Exactly one of adm-zip or yauzl is a runtime dependency of `extensions/forgejo/` (plus its `@types` as devDependency). - [ ] `npm install` completed; `package.json` + `package-lock.json` updated; no extraneous deps. - [ ] `npm run check` (tsc --noEmit) passes in `extensions/forgejo/`. - [ ] The chosen library's API (entry iteration + per-entry extraction from a Buffer) is verified with a tiny throwaway script or a jest test reading a small fixture ZIP. - [ ] The choice and rationale are documented (comment on this issue or a note in `src/actions.ts`). ### Test Plan - Run `cd extensions/forgejo && npm install && npm run check`. - Verify the dependency resolves: `node -e "require('yauzl')"` (or adm-zip) prints no error. - Create a 2-entry ZIP (`zip` CLI or the library itself) and confirm entries can be listed and extracted to an `os.tmpdir()` subdir.

david added this to the M6 - Actions tooling (runs/jobs/logs + cache) milestone

2026-08-29 03:27:51 +00:00

david added the

feature

label

2026-08-29 03:27:51 +00:00

david referenced this issue

2026-08-29 03:28:39 +00:00

src/actions.ts — downloadRunLogs (ZIP, guarded unzip, cache namespace) #116

david referenced this issue

2026-08-29 07:19:01 +00:00

issue-112: Add zip-extraction dependency (adm-zip vs yauzl) #124