Add Forgejo Actions CI (nox checks, gitleaks, WuMing review) #11
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/issue-7-forgejo-actions-ci"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Introduces CI for the repo:
.forgejo/workflows/ci.yml—checks(uv sync →uv run nox→uv build),gitleaks, and a WuMingpr-reviewjob (DeepSeek-backed), onpull_request+pushtomain+workflow_dispatch, runnerzhuirinoxfile.py—ty,ruff,complexipy, andpytestsessions (pytest on 3.12 + 3.13).pre-commit-config.yamlas the single version sourceAGENTS.md— drops the no-CI note, adds the nox rowAlso fixes two pre-existing failures that CI surfaced in
src/yunjin/web/routes/reader.pyindex(): a heterogeneous dict caused ty diagnostics, and the function exceeded the complexipy limit — refactored to aTypedDictAggregateRowplus a row-builder helper.Closes #7
WuMing
Found 8 issue(s). See inline comments below.
@ -0,0 +11,4 @@jobs:pr-review:runs-on: zhuirisecurity [HIGH]
A08/A05: The checks job runs untrusted pull-request code (uv run nox) on a self-hosted runner without restricting forks or using ephemeral isolation. Attackers can execute arbitrary commands on the runner and access its environment/network.
@ -0,0 +15,4 @@timeout-minutes: 20if: github.event_name == 'pull_request'steps:- uses: https://forge.marvin8.zone/marvin8/wuming@mainsecurity [HIGH]
A08: External action is pinned only to a mutable @main branch and receives forge.token and wuming_deepseek_key. A compromised branch could exfiltrate secrets or execute malicious code. Pin the action to a reviewed full commit SHA.
@ -12,3 +12,2 @@- Forge: https://forge.marvin8.zone/marvin8/yunjin (public; `origin` is the only remote)- **No CI is configured on this repo.** Validation is local only: run the checksbelow before committing. After opening a PR there is nothing for ci-watch to poll.- **CI runs on every PR and push to `main`** (Forgejo Actions, runner `zhuiri`):docs [LOW]
The trigger list reads as exhaustive ("on every PR and push to
main") but omitsworkflow_dispatch, which the workflow also supports per the PR description. Add it, e.g. "... on every PR, push tomain, and manualworkflow_dispatchruns ...", so readers know they can trigger CI by hand.@ -13,2 +13,2 @@- **No CI is configured on this repo.** Validation is local only: run the checksbelow before committing. After opening a PR there is nothing for ci-watch to poll.- **CI runs on every PR and push to `main`** (Forgejo Actions, runner `zhuiri`):nox checks (ty/ruff/complexipy/pytest on 3.12+3.13), gitleaks, and a WuMingdocs [MEDIUM]
The parenthetical attaches "on 3.12+3.13" to the whole list, implying ty, ruff and complexipy are version-matrixed too. Per the new noxfile.py, only the pytest session runs on 3.12 and 3.13 (ty/ruff/complexipy run once on the default interpreter). Reword for accuracy: "nox checks (ty, ruff, complexipy, and pytest on 3.12 + 3.13)".
@ -15,0 +14,4 @@nox checks (ty/ruff/complexipy/pytest on 3.12+3.13), gitleaks, and a WuMingAI review on PRs. After opening a PR, run ci-watch against it. The first CIrun downloads the full dependency tree (sentence-transformers → torch); laterruns are cheap thanks to uv's wheel cache.docs [LOW]
The claim that later runs are cheap assumes a persistent uv cache on the
zhuirirunner, but no caching step (actions/cache or a persistentUV_CACHE_DIR) is described in the workflow. Verify caching is actually configured; otherwise every run re-resolves/re-downloads the torch tree and this sentence will mislead agents about CI duration.@ -24,3 +27,3 @@| `complexipy` | Complexity check | agent || `prek run --all-files` | Pre-commit hooks | **human operator only** || `nox` | Full validation | **human operator only** || `nox` | Full validation (mirrors CI) | agent |docs [MEDIUM]
"Full validation (mirrors CI)" overstates what nox covers: CI (line 14) also runs gitleaks and the WuMing review, which the nox sessions (ty/ruff/complexipy/pytest) do not include. Since gitleaks is exposed via
prek run --all-files, which line 28 keeps human-operator-only, an agent running nox still will not cover the full CI surface. Clarify, e.g. "Full validation (same checks as the CIchecksjob; gitleaks still requires prek/human)".@ -1,6 +1,8 @@"""Reader routes for Yunjin web interface."""import sqlite3tests [LOW]
Non-trivial logic change with no accompanying test changes:
index()was refactored to delegate to the new_build_aggregate_row()helper and a newAggregateRowTypedDict. This reworks the aggregate enrichment (article counts, unread counts, tag collection, featured-image selection) and the sort key, so a regression here would silently change the reader index output. No test file appears anywhere in this diff. Add or update unit tests that exercise_build_aggregate_row()andindex()(e.g. aggregates with zero articles, with/without images, mixed read/unread state, and ordering by unread count) to lock in the refactor.@ -40,45 +97,13 @@ def index() -> str:db = get_db()user_id = 1 # Default user for now (Phase 6 will add multi-user)security [MEDIUM]
A01/A07: The route hardcodes user_id = 1 instead of deriving the current authenticated user from the session. Any request is treated as the default user, bypassing authentication and authorization once multi-user/session auth is present. Use the authenticated user identity.
AGENTS.mdline 13✅ Fixed in
250fe3f— the CI note now lists "every PR, push tomain, and manual dispatch".AGENTS.mdline 14✅ Fixed in
250fe3f— reworded sopytestalone is marked as matrixed on 3.12 + 3.13.AGENTS.mdline 17✅ Fixed in
250fe3f— removed the unverified cache claim; the note now only warns that the first run downloads the full dependency tree.AGENTS.mdline 29✅ Fixed in
250fe3f— the row now reads "Runsty,ruff,complexipy,pytest(the CI checks job)".src/yunjin/web/routes/reader.pyline 40🔴 Not actioned —
user_id = 1is a pre-existing placeholder for the multi-user work (Phase 6), carried over unchanged from the originalindex()(the comment on that line even says "Default user for now (Phase 6 will add multi-user)"). It is out of scope for this CI PR; the refactor only moved the code, it did not introduce the hardcoding.src/yunjin/web/routes/reader.pyline 3✅ Fixed in
250fe3f— addedTestReaderRoutes::test_build_aggregate_rowcovering article count, unread count, tags, and featured-image selection for the new helper..forgejo/workflows/ci.ymlline 18🔴 Not actioned —
@mainis intentional: thewumingaction is Marvin8's own action on his own forge, and this is the established pattern on his other repos (kokopelli, cang). Confirmed with Marvin8 to leave as-is..forgejo/workflows/ci.ymlline 14🔴 Not actioned — confirmed with Marvin8: this is the accepted model. PRs on this repo are reviewed by Marvin8 before merge, so CI effectively runs once the branch has been eyeballed; no fork-restriction changes at this time.
WuMing
Found 4 issue(s). See inline comments below.
@ -0,0 +26,4 @@runs-on: zhuiritimeout-minutes: 10steps:- uses: actions/checkout@v4config [LOW]
actions/checkout@v4defaults tofetch-depth: 1, so only the tip commit is present.gitleaks detectwalks git history, and with a shallow clone it can only see the single checked-out commit, missing secrets introduced/removed in earlier commits. Addwith: fetch-depth: 0(or pass--no-git/usegitleaks dirif you intentionally only want to scan the working tree).@ -0,0 +34,4 @@GITLEAKS_CHECKSUMS="gitleaks_${GITLEAKS_VERSION}_checksums.txt"curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/${GITLEAKS_FILE}" -o "/tmp/${GITLEAKS_FILE}"curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/${GITLEAKS_CHECKSUMS}" -o "/tmp/${GITLEAKS_CHECKSUMS}"cd /tmp && grep "${GITLEAKS_FILE}" "${GITLEAKS_CHECKSUMS}" | sha256sum -cconfig [HIGH]
cd /tmpis executed in the step's shell (eachrun:block is a single shell invocation), so the working directory stays/tmpfor the remaining commands. That meansgitleaks detect --source .on line 39 scans/tmpinstead of the checked-out repository, and the secret scan silently passes without ever inspecting the code. Wrap the checksum step in a subshell or use absolute paths, e.g.(cd /tmp && grep ... | sha256sum -c)and thengitleaks detect --source "$GITHUB_WORKSPACE" --verbose --redact.@ -15,0 +13,4 @@- **CI runs on every PR, push to `main`, and manual dispatch** (Forgejo Actions,runner `zhuiri`): the `checks` job (nox: `ty`, `ruff`, `complexipy`, `pytest` —`pytest` matrixed on 3.12 + 3.13), a gitleaks scan, and a WuMing AI review onPRs. After opening a PR, run ci-watch against it. The first CI run downloadsdocs [MEDIUM]
Unbalanced parenthesis. The parenthetical opened on line 13 with "(Forgejo Actions," is already closed on line 14 by the ")" after
zhuiri, and the second parenthetical "(nox: …" closes on line 15 with "3.13)". The ")" in "PRs)" therefore has no matching "(" and renders as a stray character, suggesting a dropped clause. Fix by ending the sentence with "PRs." (removing the stray ")"), or, if the intent was for the whole job list to sit inside one parenthetical, remove the ")" afterzhuirion line 14 so the pair matches.@ -40,45 +97,13 @@ def index() -> str:db = get_db()user_id = 1 # Default user for now (Phase 6 will add multi-user)security [MEDIUM]
A01: The reader route still hard-codes
user_id = 1and uses it for per-user read-state lookups without an authentication/authorization check. Once multi-user access exists, any request can read or act as the default user. Resolve the current user from the authenticated session instead of using a fixed ID..forgejo/workflows/ci.ymlline 37✅ Fixed in
d873323— split into separate "Install gitleaks" and "Run gitleaks" steps (kokopelli's known-good shape), so thecd /tmpno longer leaks into the scan step..forgejo/workflows/ci.ymlline 29✅ Fixed in
d873323— gitleaks job checkout now usesfetch-depth: 0.AGENTS.mdline 16✅ Fixed in
d873323— the parens were actually balanced, but the nested parentheticals were genuinely hard to parse, so I de-nested the sentence (the Forgejo Actions parenthetical is now its own sentence, and the checks detail has a single paren).src/yunjin/web/routes/reader.pyline 40🔴 Not actioned — same pre-existing placeholder as the earlier comment on this line; see the reply to comment 2440. Multi-user is Phase 6, out of scope for this PR.
WuMing
Found 8 issue(s). See inline comments below.
@ -0,0 +15,4 @@timeout-minutes: 20if: github.event_name == 'pull_request'steps:- uses: https://forge.marvin8.zone/marvin8/wuming@mainconfig [HIGH]
The third-party action is pinned to a mutable branch (
@main). Any force-push or compromise of that repository will execute arbitrary code in this job, which has access tosecrets.wuming_deepseek_key. Pin the action to an immutable tag or, preferably, a full commit SHA (e.g..../wuming@<40-char-sha>), and bump it deliberately.security [HIGH]
A08: The pr-review job executes a third-party action from a self-hosted Forgejo URL at the mutable @main branch. This unpinned action receives Forgejo token and DeepSeek API key credentials, so a compromised or malicious action version could exfiltrate secrets or modify the repository. Pin the action to a reviewed commit SHA and/or vendor it, and apply least-privilege credentials.
@ -0,0 +17,4 @@steps:- uses: https://forge.marvin8.zone/marvin8/wuming@mainwith:forgejo_token: ${{ forge.token }}config [HIGH]
forge.tokenis not a defined context in Forgejo Actions (there is noforgecontext; the available contexts are github, secrets, env, vars, job, jobs, steps, runner, strategy, matrix, needs, inputs). The expression will therefore not resolve to the workflow's automatic token and the WuMing step will receive an empty/invalid token. Use${{ secrets.GITHUB_TOKEN }}or${{ github.token }}.@ -0,0 +39,4 @@cd /tmp && grep "${GITLEAKS_FILE}" "${GITLEAKS_CHECKSUMS}" | sha256sum -ctar -xzf "/tmp/${GITLEAKS_FILE}" -C /usr/local/bin gitleaks- name: Run gitleaksrun: gitleaks detect --config .gitleaks.toml --source . --verbose --redactconfig [MEDIUM]
gitleaks detecthas been deprecated since gitleaks v8.19 and the job installs v8.30.1 (version taken from .pre-commit-config.yaml). On current releases it emits a deprecation warning and may be removed entirely, breaking the job. Use the replacement subcommand, e.g.gitleaks git --config .gitleaks.toml --source . --verbose --redact(orgitleaks dirfor a filesystem scan).@ -46,3 +46,2 @@- repo: https://codeberg.org/marvin8/zaojunrev: 1.0.1- repo: https://github.com/gitleaks/gitleaksconfig [LOW]
The gitleaks hook repo is pinned via
rev: v8.30.1, which pre-commit resolves by cloning the repo; the hook itself is declared upstream aslanguage: golang, so contributors must have a Go toolchain installed forpre-commit runto work locally. Since the same rev is parsed by CI to download the release binary, keep this tag in sync and consider documenting the Go requirement (or usinglanguage: system/the published binary) so the local hook and the CI job stay consistent.@ -13,2 +13,2 @@- **No CI is configured on this repo.** Validation is local only: run the checksbelow before committing. After opening a PR there is nothing for ci-watch to poll.- **CI runs on every PR, push to `main`, and manual dispatch** (Forgejo Actions,runner `zhuiri`). Jobs: `checks` (nox: `ty`, `ruff`, `complexipy`, `pytest` —docs [LOW]
The job enumeration is hard to parse: it is a comma-separated list whose first element contains a nested colon list plus an em-dash aside, so
gitleaksandpr-reviewvisually read as part of thechecksjob. Reformat as a bullet list, e.g. "Jobs:checks(ty,ruff,complexipy,pytest— pytest on 3.12 + 3.13),gitleaks,pr-review(WuMing, PRs only)."@ -15,0 +13,4 @@- **CI runs on every PR, push to `main`, and manual dispatch** (Forgejo Actions,runner `zhuiri`). Jobs: `checks` (nox: `ty`, `ruff`, `complexipy`, `pytest` —`pytest` matrixed on 3.12 + 3.13), `gitleaks`, and a WuMing AI `pr-review` onPRs. After opening a PR, run ci-watch against it. The first CI run downloadsdocs [MEDIUM]
This bullet replaced the previous instruction to validate locally before committing ("run the checks below before committing") with a purely post-PR step ("run ci-watch against it"). As written, an agent's only documented validation path is after the PR is opened, which invites pushing unvalidated code. Add an explicit pre-PR instruction, e.g. "Run
noxlocally before opening a PR; then run ci-watch against the PR once CI starts."@ -24,3 +27,3 @@| `complexipy` | Complexity check | agent || `prek run --all-files` | Pre-commit hooks | **human operator only** || `nox` | Full validation | **human operator only** || `nox` | Runs `ty`, `ruff`, `complexipy`, `pytest` (the CI checks job) | agent |docs [LOW]
Equating the
noxsession with the CIchecksjob is inaccurate: the CIchecksjob also performsuv syncanduv build, and the CIpytestsession is matrixed over 3.12 and 3.13 while a localnoxrun uses a single interpreter. Reword to avoid implying exact parity, e.g. "|nox| Runs the same checks as CI (ty,ruff,complexipy,pytest) | agent |"..forgejo/workflows/ci.ymlline 18🔴 Not actioned — same item as comment 2438, already decided with Marvin8: the
wumingaction is his own action on his own forge and@mainis the established convention across his repos. See the reply to 2438..forgejo/workflows/ci.ymlline 20🔴 Not actioned —
forgeis Forgejo's native context (the GitHub equivalent ofgithub);forge.tokenis valid on this instance. Evidence: the identical expression is used in the working CI of kokopelli and cang, and this PR's ownpr-reviewjob ran successfully withforge.tokenon the green run..forgejo/workflows/ci.ymlline 42✅ Fixed in
5e08d4a— migrated togitleaks git --config .gitleaks.toml --redact --verbose .(verified against gitleaks README:detect/protectdeprecated in v8.19.0;gitis the replacement and takes the repo path positionally)..pre-commit-config.yamlline 47🔴 Not actioned — that's standard pre-commit behaviour for every hook in this file (ruff, uv, complexipy, detect-secrets all resolve by cloning a pinned
rev). Same pattern as kokopelli/cang.AGENTS.mdline 16✅ Fixed in
5e08d4a— the note now instructs "Runuv run noxlocally before pushing" alongside the CI description.AGENTS.mdline 29✅ Fixed in
5e08d4a— the nox row now reads "the nox part of the CI checks job".AGENTS.mdline 14✅ Fixed in
5e08d4a— the three CI jobs are now a bulleted sub-list with one job per line..forgejo/workflows/ci.ymlline 18🔴 Not actioned — duplicate of 2438/2466; the
@mainpin is intentional and confirmed with Marvin8 (his own action on his own forge).WuMing
Found 6 issue(s). See inline comments below.
@ -0,0 +10,4 @@cancel-in-progress: truejobs:pr-review:config [MEDIUM]
The workflow never declares a
permissions:block. Thepr-reviewjob needs write access to the API to publish review comments; relying on the instance default token scope is fragile and grants more access than required tochecks/gitleaks. Add a top-levelpermissions: contents: readand widen only for this job, e.g.permissions: { contents: read, pull-requests: write, issues: write }.@ -0,0 +15,4 @@timeout-minutes: 20if: github.event_name == 'pull_request'steps:- uses: https://forge.marvin8.zone/marvin8/wuming@mainconfig [HIGH]
The third-party review action is pinned to the mutable branch
@mainwhile being handed a repository token and a DeepSeek API key (deepseek_key). Any upstream force-push or compromise executes arbitrary code with those secrets. Pin to an immutable ref — ideally a full 40-char commit SHA (e.g..../wuming@<sha>), or at minimum a released tag — and update it deliberately.security [HIGH]
The workflow uses an external action from a moving branch (@main) and passes secrets to it (forgejo_token, wuming_deepseek_key). This is an A08 software/data integrity issue: if the wuming repository is compromised or changed, the action can exfiltrate CI secrets. Pin the action to a full commit SHA or an immutable audited tag and minimize secret exposure.
@ -0,0 +17,4 @@steps:- uses: https://forge.marvin8.zone/marvin8/wuming@mainwith:forgejo_token: ${{ forge.token }}config [HIGH]
forgeis not a valid Forgejo/Gitea Actions context (the supported contexts aregithub,secrets,vars,env,job,steps,runner, ...). This expression resolves to an empty string, so the WuMing review action receives no Forgejo token and cannot fetch the diff or post its review. Useforgejo_token: ${{ github.token }}(or${{ secrets.GITHUB_TOKEN }}), matching thegithub.*context already used on lines 9 and 16.@ -0,0 +37,4 @@curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/${GITLEAKS_FILE}" -o "/tmp/${GITLEAKS_FILE}"curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v${GITLEAKS_VERSION}/${GITLEAKS_CHECKSUMS}" -o "/tmp/${GITLEAKS_CHECKSUMS}"cd /tmp && grep "${GITLEAKS_FILE}" "${GITLEAKS_CHECKSUMS}" | sha256sum -ctar -xzf "/tmp/${GITLEAKS_FILE}" -C /usr/local/bin gitleaksconfig [LOW]
The gitleaks tarball is extracted straight into
/usr/local/bin, which requires a privileged/root runner. If thezhuirirunner is not containerised as root this step fails with a permissions error. Extract to a workspace-local directory (tar -xzf ... -C "$HOME/.local/bin") and add it toPATH, or prependsudoonly if the runner guarantees passwordless sudo.@ -0,0 +1,8 @@[extend]useDefault = true[allowlist]config [MEDIUM]
The top-level singular
[allowlist]table is the legacy form; gitleaks v8.19+ deprecated it in favour of the plural[[allowlists]]array (and the pinned version here is v8.30.1). Depending on the build this either emits a deprecation warning or is ignored, silently disabling the.secrets.baselineexclusion so detect-secrets hashes start failing the scan. Rewrite as[[allowlists]]with the samedescription/pathskeys.