Reject API modification of internal topics

[RotemCDos]

Fixes #1967

• Breaking change? (if so, please describe the impact and migration path for existing application instances)

  Behavioural change for API consumers, not for UI users. Requests that modify an internal topic —
  and that succeed today — will start returning 400 with error code 4022
  (INTERNAL_TOPIC_MODIFICATION). The UI is unaffected, since it already hides every one of these
  actions for internal topics.

  Anyone scripting against the API to, say, adjust retention.ms on a _-prefixed topic will need
  to do it with kafka-configs.sh or another admin tool instead. There's no migration path or opt-out
  in this PR by design — see the open question below if a toggle is preferred.

  One smaller change: DELETE /topics/{topic} on a non-existent topic now returns
  TopicNotFoundException (404) rather than the admin client's error, because the topic is described
  before deletion.

What changes did you make? (Give an overview)

Internal topics can't be edited, cleared, recreated or deleted from the UI — the actions dropdown is
disabled and the rows aren't selectable — but the backend never enforced that, so the same operations
went straight through when called against the REST API directly, or via the MCP tools that share the
controllers. #1967 has the details.

This adds the missing server-side check:

• InternalTopic.isInternal(TopicDescription, prefix) — extracted from InternalTopic.from(...),
  which already computed exactly this (topicDescription.isInternal() || name.startsWith(prefix)) but
  only used it for display. Now there's one definition both the display flag and the new guard read
  from, so they can't drift apart.
• TopicsService.validateTopicIsNotInternal(...) — applied to updateTopic, deleteTopic,
  recreateTopic, cloneTopic, increaseTopicPartitions and changeReplicationFactor. It sits in
  the service rather than the controller so the MCP tools are covered by the same code path.
• MessagesService.deleteTopicMessages(...) — same rejection for "Clear messages", reusing the
  TopicDescription it already fetches, so it costs no extra call.
• InternalTopicModificationException → new ErrorCode.INTERNAL_TOPIC_MODIFICATION(4022) →
  HTTP 400.

Deliberately left alone, to match what the UI still permits:

• Creating a topic whose name starts with the internal prefix — the create form allows it.
• Producing messages to an internal topic — "Produce Message" is gated only on read-only mode.
• Cloning is rejected when the source is internal; the target name isn't checked, since that's
  a create.

In deleteTopic the internal check runs before the TOPIC_DELETION cluster-feature check, so the
reason for the rejection doesn't depend on how far cluster statistics have loaded.

Is there anything you'd like reviewers to focus on?

1. Should this be configurable? I went unconditional because the UI is unconditional, and a
   backend that quietly permits what the frontend forbids is how this bug arose. But it does remove a
   capability that someone may have been relying on, so if you'd rather have a
   kafka.allowInternalTopicModification-style property, I'm happy to add it.
2. Is the scope right? Specifically the three carve-outs above — create, produce, and clone-target.
   They match the UI exactly today, but "matches the UI" may not be the rule you want long-term.
3. Is 400 the right status? ValidationException (400) is what the neighbouring "Topic deletion
   restricted" check uses, so I followed it. ReadOnlyModeException uses 405, which is arguably the
   closer analogue — this is a permanently unavailable operation, not a malformed request.
4. The extra describeTopic call on updateTopic and deleteTopic, which didn't load the topic
   before. The other four guarded methods already called loadTopic, so they pay nothing extra. These
   are all rare, user-initiated operations, so I favoured one uniform guard over threading the flag
   through each method — say the word if you'd rather avoid the round trip.

How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)

• No need to
• Manually (please, describe, if necessary)
• Unit checks
• Integration checks
• Covered by existing automation

InternalTopicsModificationTest — 10 tests, all passing against a real Kafka via testcontainers:

• each of the seven guarded operations is rejected with InternalTopicModificationException
• DELETE and PATCH on /topics/{topic} return 400 at the endpoint level
• a non-internal topic is still modifiable, so the guard isn't over-broad

One thing the first run caught, worth noting for anyone writing similar tests: a freshly created
topic isn't immediately visible through describeTopics(), so asserting straight after
createTopic() races metadata propagation and surfaces TopicNotFoundException rather than the
rejection. The test waits for the topic to become visible before acting on it.

I also ran MessagesServiceTest, TopicsServicePaginationTest, ReadOnlyModeTests and
KafkaTopicCreateTests. Two failures show up there — `KafkaTopicCreateTests

shouldRecreateExistingTopicSuccessfully(4004002 Topic deletion restricted) and MessagesServiceTest > maskingAppliedOnConfiguredClusters (TopicNotFoundException). **Both fail identically on a clean mainin the same environment**, so they're pre-existing and unrelated to this change; I verified by re-running them onbc2d7cawith no patch applied. They look environmental (single-broker container,TOPIC_DELETION` feature not yet scraped) rather than genuine bugs, but
flagging them in case they're known flakes.

:api:compileJava, :api:compileTestJava and :api:checkstyleMain pass. :api:checkstyleTest
reports one import-order warning in RegexBasedProviderAuthorityExtractorTest.java:47 that is
present on a clean main and untouched by this PR.

Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (e.g. ENVIRONMENT VARIABLES) — no
  changes needed: no new or changed properties, and this repo carries no prose docs.
• My changes generate no new warnings (e.g. Sonar is happy)
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes — see the note above on two
  pre-existing failures that also occur on a clean main.
• Any dependent changes have been merged

Check out Contributing and Code of Conduct

Summary by CodeRabbit

• New Features
  • Added protection for internal topics, preventing deletion or modification through topic and message management actions.
  • Internal topics are recognized by Kafka metadata or a configurable name prefix.
  • Attempts to modify internal topics now return a clear HTTP 400 error.
• Bug Fixes
  • Ensured regular, non-internal topics remain fully modifiable.

[kapybro]

AI Summary

This GitHub issue addresses a security gap where internal Kafka topics (prefixed with _) could still be modified via the API, despite the UI preventing such actions. The proposed solution enforces server-side validation to reject modification attempts on internal topics, returning a 400 error with code 4022. The change is intentional and lacks a migration path, as it aligns with the UI's existing restrictions. The reviewer is asked to consider whether this should be configurable, the scope of exceptions (e.g., topic creation, producing messages), and whether 400 is the appropriate HTTP status. Integration tests confirm the fix works, though two unrelated test failures were noted.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 632b97e6-314f-47fb-aa12-5b1637ba0e6f

📥 Commits

Reviewing files that changed from the base of the PR and between 6b05776 and 86a6be8.

📒 Files selected for processing (6)

• api/src/main/java/io/kafbat/ui/exception/ErrorCode.java
• api/src/main/java/io/kafbat/ui/exception/InternalTopicModificationException.java
• api/src/main/java/io/kafbat/ui/model/InternalTopic.java
• api/src/main/java/io/kafbat/ui/service/MessagesService.java
• api/src/main/java/io/kafbat/ui/service/TopicsService.java
• api/src/test/java/io/kafbat/ui/service/InternalTopicsModificationTest.java

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The API now detects internal topics by Kafka metadata or the configured prefix. Topic and message modifications fail with a dedicated BAD_REQUEST error. Integration tests cover service operations, REST endpoints, and non-internal topics.

Changes

Internal topic protection

Layer / File(s)	Summary
Internal topic detection and error contract api/src/main/java/io/kafbat/ui/model/InternalTopic.java, api/src/main/java/io/kafbat/ui/exception/*	Centralizes internal-topic detection and adds error code 4022 with InternalTopicModificationException.
Topic and message modification guards api/src/main/java/io/kafbat/ui/service/TopicsService.java, api/src/main/java/io/kafbat/ui/service/MessagesService.java	Rejects internal-topic updates, deletion, recreation, cloning, partition changes, replication changes, and message deletion.
Integration validation api/src/test/java/io/kafbat/ui/service/InternalTopicsModificationTest.java	Verifies service errors, HTTP 400 responses, metadata retry handling, cleanup, and modification of non-internal topics.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 86a6b

The PR adds server-side protection against modifying internal topics while preserving permitted operations; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant TopicsService
  participant InternalTopic
  participant ReactiveAdminClient
  Client->>TopicsService: Request topic modification
  TopicsService->>InternalTopic: Check topic metadata and prefix
  InternalTopic-->>TopicsService: Internal or non-internal
  alt Internal topic
    TopicsService-->>Client: InternalTopicModificationException, HTTP 400
  else Non-internal topic
    TopicsService->>ReactiveAdminClient: Execute modification
    ReactiveAdminClient-->>Client: Return operation result
  end

Loading

Suggested reviewers: alexeyzavyalov

Poem

A rabbit guards the topic gate,
Underscores mark the guarded state.
Bad requests hop back in line,
Safe topics pass and changes shine.
Tests thump softly: “All is right!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: rejecting API modifications to internal topics.
Linked Issues check	✅ Passed	The changes satisfy issue #1967. They block all listed modification operations in shared services, use Kafka internal status or the configured prefix, return error code 4022 with HTTP 400, and keep to…
Out of Scope Changes check	✅ Passed	All changes support the linked issue. The new error code, exception, centralized predicate, service validation, and integration tests are directly related to blocking internal-topic modifications.

Full details: Linked Issues check

Explanation

The changes satisfy issue #1967. They block all listed modification operations in shared services, use Kafka internal status or the configured prefix, return error code 4022 with HTTP 400, and keep topic creation and message production allowed.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}