Skip to content

Dedupe observable/entity/indicator tag endpoint bodies (item #5, first slice) - #1333

Merged
tomchop merged 1 commit into
mainfrom
fix/dedupe-tag-endpoints
Jul 30, 2026
Merged

Dedupe observable/entity/indicator tag endpoint bodies (item #5, first slice)#1333
tomchop merged 1 commit into
mainfrom
fix/dedupe-tag-endpoints

Conversation

@tomchop

@tomchop tomchop commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Summary

First incremental slice of the architecture review's item #5 (the
observable/entity/indicator router triplication) — the /tag endpoint
body, one of the four behaviors the review names as extraction
candidates ("search endpoint body, tag endpoint body, context
endpoints, bulk-add loop").

The observable, entity, and indicator routers each independently
implemented the exact same /tag endpoint logic: look up every
requested ID (raising on any miss), then tag each found object and
record its resulting tags. The only genuine per-type differences were:

  • the not-found status code — observable uses 400, entity/indicator use
    404 (an existing, unintentional inconsistency — preserved here
    exactly rather than silently "fixed" as a side effect of a refactor)
  • the wording of the not-found detail message

Change

Added core/web/apiv2/tagging.py with a single tag_objects() helper,
parameterized by base type and the two per-type variation points above
— mirroring the existing pattern in core/web/apiv2/context.py
(add_context/replace_context/delete_context, already generic over
Type[T]). Each router's /tag endpoint is now a few lines that call
the helper and wrap the result in its own typed response model. The
request/response Pydantic models and route decorators are untouched in
all three routers.

OpenAPI spec verified byte-identical: generated the spec from a
throwaway build of the pre-change routers and diffed it (md5) against
the post-change build — identical.

Incidental fix: observable's /tag endpoint built its not-found
detail message without an f prefix, so on a missing ID it literally
returned the string "...ID:{observable_id}" instead of the real ID.
Preserving this exactly would have meant deliberately keeping a broken
string in the new shared helper's callback, which seemed worse than
just fixing it — so it's fixed here, called out explicitly rather than
silently bundled in. Doesn't change status code or response shape, only
the detail string content on an already-erroring request.

Test plan

  • Added test_tag_observable_unknown_id
    (tests/apiv2/observables.py) covering the incidental fix —
    verified it fails on the pre-fix router (asserts the literal
    broken string) and passes with the fix.
  • Full tests/apiv2 suite: 200/202 pass (same 2 known pre-existing
    failures in tests/apiv2/tasks.py, unrelated).
  • tests/schemas (190/190) and tests/core_tests (31/31) pass
    unchanged.
  • ty check (core+yetictl and plugins jobs): 0 errors.
  • ruff check / ruff format --check: clean.

Scope note

This is deliberately just the tag endpoint. The review's other named
candidates (search endpoint body, get-by-name, details, delete,
bulk-add) are structurally similar but each has its own small
asymmetries worth checking individually (e.g. observable's search
lacks the filter_aliases/get_multiple support entity/indicator
have) — better done as separate, individually-reviewable follow-ups
than one large PR.

…ared helper

The observable, entity, and indicator routers each independently
implemented the exact same /tag endpoint body: look up every requested
ID (404/400 on any miss), then tag each found object and record its
tags. The only real per-type differences were the not-found status
code (observable uses 400, entity/indicator use 404 -- an existing,
unintentional inconsistency, preserved here rather than silently
"fixed" as part of a refactor) and the wording of the not-found detail
message.

Add core/web/apiv2/tagging.py with a single tag_objects() helper,
parameterized by base type (mirroring the existing context.py pattern
for add_context/replace_context/delete_context) plus the two points of
per-type variation. Each router's endpoint is now a few lines that call
the helper and wrap the result in its own typed response model -- the
request/response Pydantic models and route decorators are untouched, so
the generated OpenAPI spec is byte-identical (verified via a diffed
throwaway build).

Incidental fix: observable's tag endpoint built its not-found detail
message without an f-string prefix, so it literally sent the string
"ID:{observable_id}" instead of the real ID. Fixed as part of moving
this logic into the shared helper, since preserving it faithfully in
the callback-based extraction would have required going out of the way
to keep it broken. Added a regression test for this specific message
(tests/apiv2/observables.py) -- verified it fails on the pre-fix router
and passes with the fix.

First slice of the architecture review's item #5 (observable/entity/
indicator router triplication). The recommended remaining slices --
search endpoint body, get-by-name, details, delete -- are structurally
similar candidates for the same treatment in follow-up PRs.
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