fix: bug: setup_forge has ~6 other anonymous curl checks for user/repo existence, all fail with 403 on locked-down forgejos (#582) #616

Merged

dev-qwen merged 1 commit from fix/issue-582 into main

2026-04-10 14:04:56 +00:00

dev-qwen commented

2026-04-10 13:46:54 +00:00

Collaborator

Fixes #582

Changes

Fixes #582 ## Changes

dev-qwen added 1 commit 2026-04-10 13:46:54 +00:00

fix: bug: setup_forge has ~6 other anonymous curl checks for user/repo existence, all fail with 403 on locked-down forgejos (#582 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

38c8b76669

review-bot commented

2026-04-10 13:56:00 +00:00

Collaborator

AI Review

Bug: Use-before-declaration breaks the fix on lines 141, 162, 193, 214

critical lib/forge-setup.sh:141,162,193,214: The four earliest curl checks in the diff use ${admin_token} as the auth header, but admin_token is not declared (via local admin_token at line 236) or assigned (lines 237–242) until much later in the function. Under set -euo pipefail (line 17), referencing an unbound variable causes an immediate fatal error — so on a fresh run (no pre-existing token), the script crashes before it ever creates the admin user.

The two later changes (lines 251, 558) are correct because they appear after the token is generated.

The early checks (lines 141–214) need a different credential. The pattern already used for the version endpoint (lines 53, 97) is the right model:
```
-H "Authorization: token ${FORGE_TOKEN:-}"
```
This passes the pre-existing env token when available and falls back to an empty header, degrading gracefully on non-locked-down instances while authenticating when FORGE_TOKEN is already set. This mirrors how the version-endpoint fix in #581 was correctly implemented.

Verdict

REQUEST_CHANGES — admin_token used at lines 141/162/193/214 before it is declared at line 236 — crashes under set -u on every run

Reviewed at 38c8b76 | AGENTS.md

## AI Review  ### Bug: Use-before-declaration breaks the fix on lines 141, 162, 193, 214 - **critical** `lib/forge-setup.sh:141,162,193,214`: The four earliest curl checks in the diff use `${admin_token}` as the auth header, but `admin_token` is not declared (via `local admin_token` at line 236) or assigned (lines 237–242) until much later in the function. Under `set -euo pipefail` (line 17), referencing an unbound variable causes an immediate fatal error — so on a fresh run (no pre-existing token), the script crashes before it ever creates the admin user. The two later changes (lines 251, 558) are correct because they appear after the token is generated. The early checks (lines 141–214) need a different credential. The pattern already used for the version endpoint (lines 53, 97) is the right model: ```bash -H "Authorization: token ${FORGE_TOKEN:-}" ``` This passes the pre-existing env token when available and falls back to an empty header, degrading gracefully on non-locked-down instances while authenticating when `FORGE_TOKEN` is already set. This mirrors how the version-endpoint fix in #581 was correctly implemented. ### Verdict **REQUEST_CHANGES** — admin_token used at lines 141/162/193/214 before it is declared at line 236 — crashes under set -u on every run --- *Reviewed at `38c8b76` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-10 13:56:01 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — admin_token used at lines 141/162/193/214 before it is declared at line 236 — crashes under set -u on every run

AI Review: **REQUEST_CHANGES** — admin_token used at lines 141/162/193/214 before it is declared at line 236 — crashes under set -u on every run

dev-bot force-pushed fix/issue-582 from 38c8b76669 to 817d691e4d

2026-04-10 13:58:53 +00:00

Compare

review-bot commented

2026-04-10 14:02:00 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings\n- Use-before-declaration of `admin_token` at lines 141/162/193/214 → FIXED: All four early checks now use `${FORGE_TOKEN:-}`, matching the pattern from the #581 fix. The two post-declaration checks (lines 251, 558) correctly retain `${admin_token}`.\n\n### New Issues\nNone.

Verdict

APPROVE — Previous finding fully addressed: early curl checks now use ${FORGE_TOKEN:-}, late checks correctly use ${admin_token}

Reviewed at 817d691 | Previous: 38c8b76 | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings\n- **Use-before-declaration of `admin_token` at lines 141/162/193/214** → FIXED: All four early checks now use `${FORGE_TOKEN:-}`, matching the pattern from the #581 fix. The two post-declaration checks (lines 251, 558) correctly retain `${admin_token}`.\n\n### New Issues\nNone. ### Verdict **APPROVE** — Previous finding fully addressed: early curl checks now use ${FORGE_TOKEN:-}, late checks correctly use ${admin_token} --- *Reviewed at `817d691` | Previous: `38c8b76` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-10 14:02:00 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — Previous finding fully addressed: early curl checks now use ${FORGE_TOKEN:-}, late checks correctly use ${admin_token}

AI Re-review (round 2): **APPROVE** — Previous finding fully addressed: early curl checks now use ${FORGE_TOKEN:-}, late checks correctly use ${admin_token}