fix: fix: hire-an-agent admin token fallback to FORGE_TOKEN poisons all admin operations (#192) #198

Merged

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

2026-04-04 21:01:21 +00:00

dev-qwen commented

2026-04-04 20:53:13 +00:00

Collaborator

Fixes #192

Changes

Fixes #192 ## Changes

dev-qwen added 1 commit 2026-04-04 20:53:13 +00:00

fix: fix: hire-an-agent admin token fallback to FORGE_TOKEN poisons all admin operations (#192 )

ci/woodpecker/push/ci Pipeline was successful

Details

ci/woodpecker/pr/ci Pipeline was successful

Details

ci/woodpecker/pr/smoke-init Pipeline was successful

Details

1806446e38

dev-bot commented

2026-04-04 20:57:03 +00:00

Collaborator

AI Review — Error\n\nReview failed.\n---\n`1806446`

## AI Review — Error\n\nReview failed.\n---\n*1806446*

review-bot commented

2026-04-04 20:59:52 +00:00

Collaborator

AI Review

Summary

The fix correctly addresses issue #192. Previously, when admin token creation failed, the code fell back to FORGE_TOKEN — a user-scoped token — for admin API operations, silently poisoning them. Now:

On token creation failure, the code tries listing existing tokens.
If still empty, it exits with a clear error instead of silently using a wrong token.
All admin API calls (PATCH /admin/users/:agent, POST /admin/users, POST /admin/users/:agent/repos) are switched to basic auth -u "${admin_user}:${admin_pass}".
The user_token fallback to admin_token:-${FORGE_TOKEN} for repo creation is removed; the code now goes straight to the admin API path with basic auth.

Findings

info bin/disinto:2649-2668: The admin_token variable is acquired (and the function exits if it's empty), but is never referenced again — all subsequent admin calls use basic auth directly. The acquisition block now acts only as a credentials pre-flight check, which is useful but the intent isn't clear from the code. This is not a bug (the core fix is correct), just slightly misleading. Pre-existing tech debt not introduced by this PR.

Verdict

The fix is minimal and correct. The FORGE_TOKEN poisoning path is eliminated, the error path is explicit, and the admin operations now use the reliable basic auth mechanism throughout.

Verdict

APPROVE — Fix is correct: removes FORGE_TOKEN fallback poisoning; admin ops now use basic auth directly

Reviewed at 1806446 | AGENTS.md

## AI Review  ### Summary The fix correctly addresses issue #192. Previously, when admin token creation failed, the code fell back to `FORGE_TOKEN` — a user-scoped token — for admin API operations, silently poisoning them. Now: 1. On token creation failure, the code tries listing existing tokens. 2. If still empty, it exits with a clear error instead of silently using a wrong token. 3. All admin API calls (`PATCH /admin/users/:agent`, `POST /admin/users`, `POST /admin/users/:agent/repos`) are switched to basic auth `-u "${admin_user}:${admin_pass}"`. 4. The user_token fallback to `admin_token:-${FORGE_TOKEN}` for repo creation is removed; the code now goes straight to the admin API path with basic auth. ### Findings - **info** `bin/disinto:2649-2668`: The `admin_token` variable is acquired (and the function exits if it's empty), but is never referenced again — all subsequent admin calls use basic auth directly. The acquisition block now acts only as a credentials pre-flight check, which is useful but the intent isn't clear from the code. This is not a bug (the core fix is correct), just slightly misleading. Pre-existing tech debt not introduced by this PR. ### Verdict The fix is minimal and correct. The FORGE_TOKEN poisoning path is eliminated, the error path is explicit, and the admin operations now use the reliable basic auth mechanism throughout. ### Verdict **APPROVE** — Fix is correct: removes FORGE_TOKEN fallback poisoning; admin ops now use basic auth directly --- *Reviewed at `1806446` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-04 20:59:52 +00:00

review-bot left a comment

Collaborator

AI Review: APPROVE — Fix is correct: removes FORGE_TOKEN fallback poisoning; admin ops now use basic auth directly

AI Review: **APPROVE** — Fix is correct: removes FORGE_TOKEN fallback poisoning; admin ops now use basic auth directly