Review runbook

How a maintainer reviews a PR from someone else: a new app, or a change to a package, sent by a contributor or an app’s developer. An AI agent runs this top to bottom; a human merges.

It is not run on a maintainer’s own PRs, where the PR’s account of what was tested and the CI checks are the record, and not on upstream version bumps, which CI merges after the unpack check.

How to run it: for “review PR #N”, copy the template into your working notes, then work the phases in order, filling one row at a time with verifiable evidence (command output, file:line, a hash, a URL). The output is the filled template plus a drafted PR comment. The reviewer recommends only — it never merges or publishes.

Rule of engagement — never execute untrusted code. No running install.sh, manifest build-commands, the app, npm/pip, or any shipped binary/script/ installer. Static analysis only. Download artifacts as bytes into an isolated temp dir, inspect before extracting, and run nothing inside. If a decision genuinely needs execution to resolve, escalate to a human.

The rules it applies are in the packager docs and the listing policies; this runbook is how a PR is checked against them. It layers on top of .github/workflows/pr-checks.yml (which validates descriptors, tests, dead-link-checks, and does a no-secrets fork build) and the scripts/audit-descriptor.mjs guardrails — this runbook owns the judgment calls CI can’t make. Running the app is the packager’s part, recorded in the PR against the testing playbook; the reviewer checks that record, not the app.

Provenance-based trust model

License is not the trust axis — it is compliance input only. Non-FOSS is allowed (the registry ships proprietary broker apps). Every package is an official upstream build fetched as extra-data; FlatPark doesn’t build apps from source (the rare flatpark/<app>-release build is a maintainer’s call, outside this runbook). What matters is where the bytes you run come from, and whether what is shipped does anything obviously malicious. The review doesn’t audit the app’s source; a full audit would cost far more than it finds.

Every acceptable submission meets the base requirements:

1. Official download source — every extra-data/archive URL is on the vendor’s official domain or the genuine upstream repository, not the submitter’s account or a mirror.
2. Unmodified repackage — build-commands only install the wrapper, desktop file, metainfo, icon and an apply_extra that unpacks the official download; no patch, sed or recompile of the payload; the shipped artifact is what the official URL serves. External sandbox adaptation is allowed: wrapper env vars, a library the runtime lacks (from a shared flatpark/prebuilt stack or a second extra-data source — not a per-app build into the ref; see Phase 3), PATH scripts that forward host commands — review those as code, since they run. Code injected into the app process (an LD_PRELOAD shim, a patched or replaced upstream file) is not external adaptation: an open-source app that needs it is rejected (the fix belongs upstream), and a proprietary app may carry it only when its metainfo description says plainly what is injected and why (com.schwab.thinkorswim).
3. Pinned bytes — sha256 (+ size for extra-data).

Then place it in a tier:

• Tier 1 — open-source app. The base requirements, plus: nothing injected and no file changed (the fix belongs upstream), and the IOC scan of Phase 5 over what is shipped. Glance at the source for obvious red flags when something in the artifact looks wrong; it is not audited.
• Tier 2 — closed-source app. The base requirements only: provenance is the whole check, since the code can’t be read.
• Tier 3 — opaque third-party / submitter-built binary. Not an official upstream release (PR #13). Reject.

Phases

Phase 0 — Rules of engagement (safety)

Static-only; nothing executed. Artifacts downloaded as bytes into an isolated temp dir; inspect-before-extract (traversal/symlink/setuid); extract --no-same-owner --no-same-permissions; run nothing. Read-only gh. Execution needed to decide → escalate to a human.

Phase 1 — Scope & classification

Classify: new app (registry/<id>/) / existing-app change / infrastructure. High-scrutiny surface (STOP if anyone but a maintainer touches it): .github/, scripts/, config/, signing config, and — for an app that already exists on the base commit — its resolve-update.sh or update.command. These are CI-executed on a repo-write token (update-check.yml runs contents: write + pull-requests: write; check-updates.sh evals update.command).

A resolver added inside a brand-new registry/<id>/ directory is the normal submission flow, not a high-scrutiny event: every new app must bring one, its blast radius is that app’s own pin, and it only executes post-merge on main. It still gets read as code in Phase 4.2 — the distinction is that a contributor may bring their own resolver but never edit one the repo already trusts. Identify the changed app id(s).

Phase 2 — Submitter & provenance

Account age, history, social graph, other repos, prior contributions. Source-repo age and corroboration; commit identity (watch for malformed emails); timeline plausibility (account → repo → release within hours is a red flag). Emit a trust tier: established / unknown-but-plausible / throwaway-suspicious. Assign the artifact provenance tier (1/2/3).

Phase 3 — Descriptor & manifest static review

• Source types: extra-data → sha256 + non-zero size; archive → only a flatpark/prebuilt stack the catalog already consumes as an archive, with sha256; file → local, reviewed as part of the PR; git or anything else → reject.
• What may land bytes in /app (→ R2): only a shared support stack released from flatpark/prebuilt that the catalog already consumes as a pinned type: archive module (ayatana-stack, mpv-stack, libxdo, wemeet-screenshare-hook). Other stacks, appimage-tools included, are extra-data. The app payload and any single-app dependency must be extra-data (fetched at install, never baked into the ref). A per-app git/archive/remote source built into /app is an auto-reject; a buildsystem: simple module that only cps in-repo wrapper/desktop/metainfo files is fine (no remote bytes).
• finish-args risk scan: escape perms (--talk-name=org.freedesktop.Flatpak, --filesystem=host, --filesystem=/) are rejected unless declared in policy.dangerous_permissions with a justification; declared ones go to a maintainer (NEEDS-HUMAN), who grants them when the app’s feature needs them. Warn on --device=all, --filesystem=home, needless --share=network.
• Optional capabilities are opt-in, not pre-granted: a broad permission that the app’s core feature doesn’t need must be absent from finish-args and instead documented as a flatpak override command in the metainfo description (see org.electerm.Electerm). A broad grant kept in finish-args needs a written justification in the PR body → warn if missing.
• What was tested: the PR body says honestly what the submitter built, installed, launched and used, and what they couldn’t test (testing playbook). Missing → warn (the reviewer never runs it; see Phase 0).
• policy: block vs reality: proprietary accurate; dangerous_permissions covers the high-risk finish-args actually used.
• build-commands: read every line; flag network fetch, curl|bash, writes outside the build dir, and any modification of the vendor payload/behavior.
• source URLs resolve to genuine upstream (no lookalike / fork).
• app-id reverse-DNS matches the real vendor (impersonation / typosquat).
• update.command is a simple relative script path (e.g. ./resolve-update.sh).
• download host = official vendor domain / upstream, not the submitter’s personal namespace.

Phase 4 — Runtime-behavior scan

Grep manifest + shipped scripts + source for runtime fetch-and-exec (npm install, pip install, curl|wget, nc, download-then-run). Allowed (vendor self-updater exception): the vendor’s own installer/updater that downloads into the app’s data/cache dir, when packaging does not patch the official behavior, writes stay in the per-app data/cache dir (no host-home), endpoints are the vendor’s own, and there is no shell pipeline from an arbitrary URL. Rejected: runtime fetch-and-exec of arbitrary/unpinned third-party code as a core mechanism (PR #13’s npm install peerflix), or an updater whose packaging patches official behavior. Review update.command/resolve scripts as code. Note network endpoints and broad filesystem×network combos.

Phase 5 — Artifact / binary provenance

• sha256 of the artifact == manifest pin.
• Inspect without execution, per artifact type: tar/tar.gz/tgz/tar.zst → list entries, reject traversal/symlink-escape/setuid; zip → list with zip tooling, same checks; deb/rpm → inspect payload without running maintainer scripts (review those statically); shell installer (.sh) → static review where practical, else treat as an opaque official prebuilt needing stronger provenance + human judgment; AppImage → accepted: it must be cracked offline, never executed — apply_extra runs flatpark/prebuilt’s appimage-tools (appimage-offset + unsquashfs -o <offset>) against the appended SquashFS, with no libfuse and no ./*.AppImage invocation. Reject an AppImage recipe that runs the stub, wants FUSE, or --extracts by executing it.
• [Tier 1] nothing injected, no vendor file changed.
• shipped artifact == official download (re-fetch + hash-compare); build did not alter the payload.
• Any bundled third-party runtime (node, electron, a JRE, …) hash-verified against the real upstream where a canonical artifact exists (compare code sections / build-id, as done for node); one that can’t be matched → NEEDS-HUMAN, not PASS.
• IOC scan: persistence (autostart/cron/systemd/.bashrc), miners (xmrig/stratum+), reverse shells (/dev/tcp, nc -e), hardcoded IPs, embedded second ELF, RPATH/RUNPATH injection.

Phase 6 — Compliance & purpose

Non-FOSS / proprietary is allowed — not a rejection reason. Reject only for purpose/legality: piracy, malware or abuse tooling, trademark-infringing impersonation, content illegal to distribute.

Phase 7 — Verdict & report

Decide per the rubric. Produce the filled template and a drafted PR comment. Reviewer recommends only — a human merges.

Decision rubric

AUTO-REJECT (any one):

• Compliance / legality violation (piracy, malware, trademark, illegal-to-distribute).
• Sandbox-escape permission (--talk-name=org.freedesktop.Flatpak, --filesystem=host//) not declared in policy.dangerous_permissions with a justification (declared → NEEDS-HUMAN).
• A git source; an unpinned source (archive/extra-data without sha256; extra-data size: 0).
• Tier 3 provenance (opaque third-party / submitter-built binary).
• [Tier 1] code injected or a vendor file changed in an open-source app.
• Download source not official, OR packaging patches/recompiles the payload, OR shipped artifact ≠ official download. (External adaptation — wrapper env, extra library modules, PATH scripts — is not a modification; review it as code.)
• Code injected into the app process (LD_PRELOAD shim, patched/replaced upstream file) in an open-source app; or in a proprietary app without a plain metainfo disclosure of what is injected and why.
• AppImage recipe that executes the stub or needs libfuse (offline unsquashfs via flatpark/prebuilt appimage-tools is the only accepted path).
• A type: archive / type: git / remote source landing bytes in /app that is not a shared support stack released from flatpark/prebuilt. App payloads and single-app dependencies must be extra-data — they must not be baked into the ref / shipped from R2. (Source-building one app’s missing library into /app is this reject.)
• Runtime fetch-and-exec of arbitrary / unpinned code as a core mechanism (the vendor self-updater exception does not count).
• update.command that is not a simple relative script path.
• IOC found (traversal / setuid / persistence / miner / reverse-shell / embedded payload / RPATH injection).
• Infra/workflow tampering from an external contributor, or an edit to an existing app’s resolver (a resolver in a brand-new app dir is not tampering).
• Throwaway account + Tier-3 binary (the PR #13 combination).

NEEDS-HUMAN: bundled runtimes that can’t be matched to upstream · novel permission needing justification · borderline provenance · unknown source type · shell installer that can’t be statically reviewed.

PASS: clean + trusted + provenance verified for its tier.

Review template

Copy this per PR and fill it in. Legend: ✅ pass · ❌ fail (hard-reject) · ⚠️ warn (needs justification) · 👤 needs-human · ➖ N/A. Tier: 1 = open source · 2 = closed source · ★ = all.

Every ❌ carries verifiable evidence; the verdict follows mechanically from “any hard-fail row hit”.