CRITICAL: delete_label/update_label address labels by name, not id -- delete_label('395') silently deleted an unrelated label #19

New issue

Open

opened 2026-10-01 16:40:36 +02:00 by kade · 0 comments

kade commented

2026-10-01 16:40:36 +02:00

Owner

Severity: critical (destructive, silent)

LabelsModule.delete_label and update_label interpolated the caller's name
into a path segment that Forgejo addresses by id:

# before (src/forgejo_client/modules/labels.py)
self.client._make_request(f"{self.config.repo_url}/labels/{name}", method="DELETE", ...)

Forgejo's spec is unambiguous (GET swagger.v1.json):

endpoint	key
`DELETE /repos/{owner}/{repo}/labels/{id}`	id
`PATCH /repos/{owner}/{repo}/labels/{id}`	id
`DELETE /repos/{owner}/{repo}/issues/{index}/labels/{identifier}`	id or name

The client conflated the issue-attachment identifier convention with label CRUD.

Impact

Ordinary name (bug): /labels/bug -> 404 -> delete_label returns False.
Non-functional for every label.
Name containing / (ci/cd, a real label in this repo): /labels/ci/cd -> 404.
Name that looks like an integer: /labels/395 -> 204, and deletes the label
whose id is 395, which has nothing to do with the name. Reports success.

Live confirmation (this is not theoretical)

While probing this on kade/forgejo-client, a label literally named "395"
(id 642) was created and DELETE .../labels/395 was issued -- the exact call the
client makes. It returned 204 and deleted id 395, an unrelated label. That
label was hard-deleted: no label row, no issue_label reference, no
action log entry (Forgejo records no label ops), and no backup on the host. It
is unrecoverable. No issue referenced it, so no issue lost a label; this repo now
has 51 labels instead of 52, with one definition (id 395, name/colour unknown)
permanently absent from the curated set.

Fix (implemented in the working tree)

Resolve the name to an id before the request, via the issues module's existing
_resolve_label_ids (already raises on unknown and on duplicate names):

(label_id,) = self.client.issues._resolve_label_ids([name])
self.client._make_request(f"{self.config.repo_url}/labels/{label_id}", method="DELETE", ...)

Tests (tests/test_labels.py, new)

test_numeric_label_name_does_not_target_the_matching_id -- the exact fixture
(label id 395 named telemetry, plus a label named "395" id 642) asserts the
DELETE URL ends /labels/642 and never /labels/395.
test_name_containing_a_slash_does_not_add_a_path_segment
test_update_label_resolves_name_to_id
test_unknown_name_raises_and_sends_no_mutation, test_unknown_numeric_name_raises_...,
test_duplicate_names_raise_...

All fail against the pre-fix code (10/12 in the new file fail on HEAD).

## Severity: critical (destructive, silent) `LabelsModule.delete_label` and `update_label` interpolated the caller's **name** into a path segment that Forgejo addresses by **id**: ```python # before (src/forgejo_client/modules/labels.py) self.client._make_request(f"{self.config.repo_url}/labels/{name}", method="DELETE", ...) ``` Forgejo's spec is unambiguous (`GET swagger.v1.json`): | endpoint | key | |---|---| | `DELETE /repos/{owner}/{repo}/labels/{id}` | **id** | | `PATCH /repos/{owner}/{repo}/labels/{id}` | **id** | | `DELETE /repos/{owner}/{repo}/issues/{index}/labels/{identifier}` | id *or* name | The client conflated the issue-attachment `identifier` convention with label CRUD. ## Impact - **Ordinary name** (`bug`): `/labels/bug` -> 404 -> `delete_label` returns `False`. Non-functional for every label. - **Name containing `/`** (`ci/cd`, a real label in this repo): `/labels/ci/cd` -> 404. - **Name that looks like an integer**: `/labels/395` -> **204, and deletes the label whose id is 395**, which has nothing to do with the name. Reports success. ## Live confirmation (this is not theoretical) While probing this on `kade/forgejo-client`, a label literally named `"395"` (id 642) was created and `DELETE .../labels/395` was issued -- the exact call the client makes. It returned **204 and deleted id 395**, an unrelated label. That label was hard-deleted: no `label` row, no `issue_label` reference, no `action` log entry (Forgejo records no label ops), and no backup on the host. It is unrecoverable. No issue referenced it, so no issue lost a label; this repo now has 51 labels instead of 52, with one definition (`id 395`, name/colour unknown) permanently absent from the curated set. ## Fix (implemented in the working tree) Resolve the name to an id before the request, via the issues module's existing `_resolve_label_ids` (already raises on unknown and on duplicate names): ```python (label_id,) = self.client.issues._resolve_label_ids([name]) self.client._make_request(f"{self.config.repo_url}/labels/{label_id}", method="DELETE", ...) ``` ## Tests (tests/test_labels.py, new) - `test_numeric_label_name_does_not_target_the_matching_id` -- the exact fixture (label id 395 named `telemetry`, plus a label named `"395"` id 642) asserts the DELETE URL ends `/labels/642` and never `/labels/395`. - `test_name_containing_a_slash_does_not_add_a_path_segment` - `test_update_label_resolves_name_to_id` - `test_unknown_name_raises_and_sends_no_mutation`, `test_unknown_numeric_name_raises_...`, `test_duplicate_names_raise_...` All fail against the pre-fix code (10/12 in the new file fail on HEAD).

kade added the

pkg:ai

label

2026-10-01 16:40:36 +02:00