ElementStore::remove_label retains the wrong entries — label map gets corrupted #669

New issue

Open

opened 2026-07-05 03:22:37 +02:00 by Hermes · 0 comments

Hermes commented

2026-07-05 03:22:37 +02:00

Collaborator

Severity: 🔴 High

File: Crates/orbital_element/src/store.rs:167-171

Bug:
The remove_label method uses .retain() which keeps entries where the closure returns true. The current closure keeps labels that are in the removal list — so it removes everything else, including labels from other elements entirely.

pub fn remove_label(&mut self, element_label: &str, labels_to_be_removed: Vec<String>) {
    if let Some(element_id) = self.label_map.get(element_label).cloned() {
        self.label_map
            .retain(|k, v| element_id.eq(v) && labels_to_be_removed.contains(k));
            // ^ retains the labels that SHOULD be removed — backward logic!
    }
}

Impact: Calling remove_label corrupts the label map: labels from other elements get wiped, and only the ones being "removed" survive for the target element.

Example: Element "A" has labels ["a", "b", "c"], element "B" has ["d"]. Call remove_label("A", ["a"]). Result: label map contains only "a" → A_id — all other labels from A and B are gone.

Fix: Negate the condition: !(element_id.eq(v) && labels_to_be_removed.contains(k))

@SakulFlee

**Severity:** 🔴 High **File:** `Crates/orbital_element/src/store.rs:167-171` **Bug:** The `remove_label` method uses `.retain()` which **keeps** entries where the closure returns `true`. The current closure keeps labels that **are** in the removal list — so it **removes everything else**, including labels from other elements entirely. ```rust pub fn remove_label(&mut self, element_label: &str, labels_to_be_removed: Vec<String>) { if let Some(element_id) = self.label_map.get(element_label).cloned() { self.label_map .retain(|k, v| element_id.eq(v) && labels_to_be_removed.contains(k)); // ^ retains the labels that SHOULD be removed — backward logic! } } ``` **Impact:** Calling `remove_label` corrupts the label map: labels from other elements get wiped, and only the ones being "removed" survive for the target element. **Example:** Element "A" has labels `["a", "b", "c"]`, element "B" has `["d"]`. Call `remove_label("A", ["a"])`. Result: label map contains only `"a" → A_id` — all other labels from A **and B** are gone. **Fix:** Negate the condition: `!(element_id.eq(v) && labels_to_be_removed.contains(k))` @SakulFlee

Hermes added the

Difficulty: Medium

Context: Bug

labels

2026-07-05 03:22:37 +02:00