Skip to content

fix: [objects] Add source object id to add_object_reference - #27

Draft
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/object-reference
Draft

elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/object-reference

Conversation

@elhoim

@elhoim elhoim commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

BLUF — add_object_reference now identifies the source object, so the call stops failing on a real MISP server.

  • Problem — MispClient::add_object_reference() posted to objectReferences/add with no source object id; MISP resolves the source only from a URL path segment or a top-level object_uuid, never from the nested body, so the request always failed.
  • Fix — Added an explicit object_id: i64 parameter placed on the path as objectReferences/add/{object_id}, mirrored it in the blocking client, and corrected the wiremock test.
  • Effect — Object references can be created, and the signature matches the two-argument form docs.md documented but the code never implemented.
  • CI — Red for repo-level reasons unrelated to this PR: the job ran on ubuntu-22.04 (no php8.3 mod_php) and test_event_crud_lifecycle raced the publish worker; both fixed in PR fix: [ci] Run integration tests on ubuntu-24.04 to match php8.3 #53.

Finding

add_object_reference omits the source object id
MispObjectReference::new() never sets object_id and the field is skip_serializing_if, so the POST omits the source object. docs.md:405 documents a two-argument (object_id, reference) form that does not exist.

Change

MispClient::add_object_reference() posted to objectReferences/add with the body wrapped as {"ObjectReference": ...} and no way to identify the source object — MISP's controller resolves the source object only from a URL path segment or a top-level object_uuid, and never reads object_id out of a nested payload (it overwrites it server-side anyway). The request would always fail with NotFoundException on a real server. Fixed by adding an explicit object_id: i64 parameter and putting it on the URL path (objectReferences/add/{object_id}), matching the two-arg signature docs.md already documented but the code never implemented. Mirrored the change in the blocking client wrapper and corrected the existing wiremock test in place to assert the real path.

Upstream evidence

MISP core app/Controller/ObjectReferencesController.php:18-27 — add($objectId) resolves the source Object from the URL path param, or (if empty) from a top-level object_uuid in the raw POST data, throwing NotFoundException('No object defined.') otherwise; lines 49-51 — if request.data has no 'ObjectReference' key it wraps the whole body as ObjectReference (so a wrapped body is fine once $objectId is resolved); lines 57-68 — 'object_id'/'object_uuid'/'source_uuid' are always set server-side from the already-resolved $object, so any object_id the client puts in the JSON body is ignored. PyMISP pymisp/api.py:937 + PyMISP pymisp/mispevent.py:723-733 — PyMISP instead posts the MISPObjectReference flat (not wrapped) with object_uuid at the top level, which is why it can hit the bare objectReferences/add path successfully; RustMISP's struct has no object_uuid field, so replicating that path was out of scope — using the URL path segment (which the controller equally accepts, per Validation::uuid($objectId) branch) is the minimal correct fix and matches docs.md:404's already-documented two-arg signature.

Verified against MISP core 843b6744 and PyMISP e7debb4f.

Tests

  • add_object_reference_sends_source_object_id_in_path (corrected in place from add_object_reference_sends_wrapped in src/client.rs) — asserts POST /objectReferences/add/42 with the wrapped ObjectReference body; fails against the pre-fix one-arg signature/wrong path, passes after the fix

Note on the original finding

Partially accurate but mis-located the mechanism. The finding correctly flagged that the source object identity never reached the request, but its diagnosis (skip_serializing_if hides object_id in the JSON body) and suggested fix (make MispObjectReference::new set object_id so the POST body carries it) would not have fixed anything: MISP's controller never reads object_id/object_uuid out of the (possibly nested) request body when resolving the source object — it only looks at the URL path segment or a top-level object_uuid, and always overwrites those fields server-side afterward. The real, larger defect is that the original call site supplied no source-object identifier via any channel MISP actually reads, so every call would 404/NotFoundException on a live server. The docs.md claim in the finding was accurate: it already documented the correct two-argument (object_id, reference) form, which the code simply never implemented.

Caveats

This is a breaking public API signature change on both MispClient::add_object_reference and MispClientBlocking::add_object_reference (now take an explicit object_id: i64 first argument) — intentional, and it makes the code match what docs.md:404 already promised. cargo build/test/clippy were not run here per the task's ground rules (centrally serialized on a shared target dir); correctness rests on the upstream controller/PyMISP citations above and manual verification that all three call sites (client.rs, client_blocking.rs, the test) agree on the new signature.


Verified locally: cargo fmt --check, cargo clippy --all-targets --all-features -- -D warnings, cargo test --all-features --lib, --test integration_tests --no-run, and cargo check --examples, each in an isolated target directory.

🤖 Generated with Claude Code

MispClient::add_object_reference() posted to `objectReferences/add`
with the body wrapped as {"ObjectReference": ...} and no way to
identify the source object. MISP's ObjectReferencesController::add()
(app/Controller/ObjectReferencesController.php:18-27) resolves the
source object only from the URL path segment ($objectId) or, if that
is empty, from a top-level `object_uuid` in the POST body - it never
reads object_id/object_uuid out of a nested ObjectReference payload,
and overwrites those fields server-side from the resolved object
regardless (lines 57-68). So the original finding's suggested fix
(stop skip_serializing_if-ing object_id in the JSON body) would not
have fixed anything: that field is not read there. The actual defect
is that the request never identified the source object at all, so it
would 404/NotFoundException("No object defined.") on any real server.

PyMISP avoids this by posting the reference flat (not wrapped) with
`object_uuid` at the top level (pymisp/api.py:937,
pymisp/mispevent.py:723-733). RustMISP's MispObjectReference has no
object_uuid field, so the minimal fix here is to take the source
object id explicitly and put it on the URL path, matching the
signature docs.md already documented at line 404
(`add_object_reference(object_id: i64, reference: &MispObjectReference)`)
but that the code never implemented.

Corrected the existing wiremock test in place (it asserted the old,
unreachable path) to assert POST /objectReferences/add/{id}.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JoSButbVaWEtGMXwTx73BZ
@elhoim
elhoim marked this pull request as draft September 3, 2026 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant