Repository navigation
Conversation
…sable
toggleEnable/{id} always flips the noticelist's current enabled state and
ignores the request body (NoticelistsController.php:133,
`$enable = (int)!$noticelist['Noticelist']['enabled']`), so
enable_noticelist and disable_noticelist were behaviourally identical:
calling disable on an already-disabled list would enable it.
Switch to the explicit set-state action, matching PyMISP
(pymisp/api.py:1686, 1697): enable posts to
`noticelists/enableNoticelist/{id}/true`, disable posts to
`noticelists/enableNoticelist/{id}` with the trailing segment omitted
entirely rather than passed as the literal string "false" (PHP treats a
non-empty URL segment as truthy, so `enableNoticelist($id, $enable=false)`
per NoticelistsController.php:153-163 must be reached via its default
argument, not a literal "false").
Corrected the wiremock tests, which previously matched the same
`/noticelists/toggleEnable/5` path for both enable and disable and so
could not have caught this bug; they now assert distinct paths and an
empty JSON body for each.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JoSButbVaWEtGMXwTx73BZ
elhoim
marked this pull request as draft
September 3, 2026 05:04
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BLUF — Use MISP's explicit
enableNoticelistaction so disabling a noticelist cannot enable it.enable_noticelistanddisable_noticelistboth posted tonoticelists/toggleEnable/{id}, which MISP implements as an unconditional flip that ignores the body, making the two methods behaviourally identical.noticelists/enableNoticelist/{id}/{enable}, matching PyMISP: enable posts to.../{id}/true, disable omits the trailing segment rather than sending literalfalse, which PHP would treat as truthy.ubuntu-22.04(no php8.3 mod_php) andtest_event_crud_lifecycleraced the publish worker; both fixed in PR fix: [ci] Run integration tests on ubuntu-24.04 to match php8.3 #53.Finding
enable/disable_noticelist both hit toggleEnable which always flips
toggleEnable computes $enable=(int)!$noticelist['enabled'] and ignores the body, so disable on an already-disabled list enables it; the two functions are behaviourally identical.
Change
enable_noticelist and disable_noticelist both posted to noticelists/toggleEnable/{id}, which the MISP controller implements as an unconditional flip that ignores the request body — making the two client methods behaviourally identical (disable on an already-disabled list would enable it). Changed both to use the explicit set-state action noticelists/enableNoticelist/{id}/{enable}, matching PyMISP: enable posts to enableNoticelist/{id}/true, disable posts to enableNoticelist/{id} with the trailing segment omitted (not passed as literal "false", which PHP would treat as truthy).
Upstream evidence
MISP NoticelistsController.php:133
$enable = (int)!$noticelist['Noticelist']['enabled'];(toggleEnable always flips, body ignored — lines 115-147); enableNoticelist explicit action at lines 153-165 ($this->Noticelist->saveField('enabled', $enable), defaulting $enable=false). PyMISP pymisp/api.py:1686f'noticelists/enableNoticelist/{noticelist_id}/true'(enable) and :1697f'noticelists/enableNoticelist/{noticelist_id}'(disable, no trailing segment).Verified against MISP core
843b6744and PyMISPe7debb4f.Tests
enable_noticelist_posts (corrected: now asserts POST /noticelists/enableNoticelist/5/true with empty JSON body)disable_noticelist_posts (corrected: now asserts POST /noticelists/enableNoticelist/5 with empty JSON body)Caveats
enableNoticelist in the MISP controller has no _isRest branch and always issues a 302 redirect to noticelists/view/{id} rather than a JSON response; this works transparently because both PyMISP's requests and this client's reqwest follow redirects by default (no Policy::none() is set in MispClientBuilder), so behavior stays parity-correct, but a wiremock test can only assert the outgoing request, not that a live MISP's redirect is followed correctly — that path is unverified here.
Verified locally:
cargo fmt --check,cargo clippy --all-targets --all-features -- -D warnings,cargo test --all-features --lib,--test integration_tests --no-run, andcargo check --examples, each in an isolated target directory.🤖 Generated with Claude Code