From a5c34a5ebabcc69823f2779ee29bc4300a48e31c Mon Sep 17 00:00:00 2001 From: Agent Date: Sun, 5 Apr 2026 22:07:53 +0000 Subject: [PATCH] fix: address PR #264 review feedback - Fix token cleanup to use bot user's Basic Auth instead of admin token (prevents silent failures when admin token auth is rejected) - Fix error message to reference correct variable (org_name/ops_name) - Add idempotency test to smoke-init.sh (runs init twice) --- bin/disinto | 7 ++++--- tests/mock-forgejo.py | 33 +++++++++++++++++++++++++++++++++ tests/smoke-init.sh | 12 ++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/bin/disinto b/bin/disinto index 1d2ecd1..4d27e38 100755 --- a/bin/disinto +++ b/bin/disinto @@ -875,9 +875,10 @@ setup_forge() { # Generate token via API (basic auth as the bot user — Forgejo requires # basic auth on POST /users/{username}/tokens, token auth is rejected) # First, try to delete existing tokens to avoid name collision + # Use bot user's own Basic Auth (we just set the password above) local existing_token_ids existing_token_ids=$(curl -sf \ - -H "Authorization: token ${admin_token}" \ + -u "${bot_user}:${bot_pass}" \ "${forge_url}/api/v1/users/${bot_user}/tokens" 2>/dev/null \ | jq -r '.[].id // empty' 2>/dev/null) || existing_token_ids="" @@ -885,7 +886,7 @@ setup_forge() { if [ -n "$existing_token_ids" ]; then while IFS= read -r tid; do [ -n "$tid" ] && curl -sf -X DELETE \ - -H "Authorization: token ${admin_token}" \ + -u "${bot_user}:${bot_pass}" \ "${forge_url}/api/v1/users/${bot_user}/tokens/${tid}" >/dev/null 2>&1 || true done <<< "$existing_token_ids" fi @@ -1071,7 +1072,7 @@ setup_ops_repo() { actual_ops_slug="${org_name}/${ops_name}" echo "Ops repo: ${actual_ops_slug} created on Forgejo (via admin API)" else - echo "Error: failed to create ops repo '${actual_ops_slug}' (HTTP ${http_code})" >&2 + echo "Error: failed to create ops repo '${org_name}/${ops_name}' (HTTP ${http_code})" >&2 return 1 fi fi diff --git a/tests/mock-forgejo.py b/tests/mock-forgejo.py index 4691072..c65b522 100755 --- a/tests/mock-forgejo.py +++ b/tests/mock-forgejo.py @@ -135,6 +135,7 @@ class ForgejoHandler(BaseHTTPRequestHandler): # Users patterns (r"^users/([^/]+)$", f"handle_{method}_users_username"), (r"^users/([^/]+)/tokens$", f"handle_{method}_users_username_tokens"), + (r"^users/([^/]+)/tokens/([^/]+)$", f"handle_{method}_users_username_tokens_token_id"), (r"^users/([^/]+)/repos$", f"handle_{method}_users_username_repos"), # Repos patterns (r"^repos/([^/]+)/([^/]+)$", f"handle_{method}_repos_owner_repo"), @@ -307,6 +308,38 @@ class ForgejoHandler(BaseHTTPRequestHandler): tokens = [t for t in state["tokens"].values() if t.get("username") == username] json_response(self, 200, tokens) + def handle_DELETE_users_username_tokens_token_id(self, query): + """DELETE /api/v1/users/{username}/tokens/{id}""" + # Support both token auth and basic auth + username = require_token(self) + if not username: + username = require_basic_auth(self) + if not username: + json_response(self, 401, {"message": "invalid authentication"}) + return + + parts = self.path.split("/") + if len(parts) >= 8: + token_id_str = parts[7] + else: + json_response(self, 404, {"message": "token not found"}) + return + + # Find and delete token by ID + deleted = False + for tok_sha1, tok in list(state["tokens"].items()): + if tok.get("id") == int(token_id_str) and tok.get("username") == username: + del state["tokens"][tok_sha1] + deleted = True + break + + if deleted: + self.send_response(204) + self.send_header("Content-Length", 0) + self.end_headers() + else: + json_response(self, 404, {"message": "token not found"}) + def handle_POST_users_username_tokens(self, query): """POST /api/v1/users/{username}/tokens""" username = require_basic_auth(self) diff --git a/tests/smoke-init.sh b/tests/smoke-init.sh index d60aed1..a8371bd 100644 --- a/tests/smoke-init.sh +++ b/tests/smoke-init.sh @@ -175,6 +175,18 @@ else fail "disinto init exited non-zero" fi +# ── Idempotency test: run init again ─────────────────────────────────────── +echo "=== Idempotency test: running disinto init again ===" +if bash "${FACTORY_ROOT}/bin/disinto" init \ + "${TEST_SLUG}" \ + --bare --yes \ + --forge-url "$FORGE_URL" \ + --repo-root "/tmp/smoke-test-repo"; then + pass "disinto init (re-run) completed successfully" +else + fail "disinto init (re-run) exited non-zero" +fi + # ── 4. Verify Forgejo state ───────────────────────────────────────────────── echo "=== 4/6 Verifying Forgejo state ==="