Skip to content

fix(interop)!: reject reserved namespaces ns and nsapi (LAB-5876) - #89

Open
27Bslash6 wants to merge 1 commit into
mainfrom
lab-5876-reserve-ns-nsapi-interop-namespace
Open

27Bslash6 wants to merge 1 commit into
mainfrom
lab-5876-reserve-ns-nsapi-interop-namespace

Conversation

@27Bslash6

@27Bslash6 27Bslash6 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Reserves ns and nsapi as interop namespaces in both the runtime key builder and the #[cachekit] proc macro. The CachekitIO server parses keys that begin with ns: or nsapi: as namespace-prefixed, so interop keys with these namespaces could not be used safely against it.

Modified Public APIs

  • cachekit::interop::interop_key now returns CachekitError::InvalidKey when namespace is exactly ns or nsapi. Its # Errors doc section documents this.
  • #[cachekit(namespace = ...)] now fails at compile time for the same two values, and the error is spanned to the string literal. The attribute docs describe the reservation.

The operation argument and the interop = ... attribute are unchanged.

Behavioral Details

  • Check order: The segment grammar is validated first and the reservation second, in both validate_segment and parse_segment. A malformed segment still produces the existing grammar error, and the reservation error only appears for well-formed input.
  • Case: Uppercase variants such as NS never reach the reservation check, because the grammar only allows lowercase.
  • Exact match only: Near-miss values stay valid. Tests cover nsx, nsapi2, nsfw, n and ns-api.
  • Error message: Both layers use the same wording: namespace "<value>" is reserved: the CachekitIO server parses a key starting "<value>:" as namespace-prefixed. The runtime message is prefixed with interop.
  • Refactor: Both validators now use early returns instead of an if/else expression. Grammar behavior is unchanged.

Maintenance Notes

  • The drift-warning comments in interop.rs and cachekit-macros now cover parse_segment as well as segment_is_valid. Any future change to the reservation has to be made in both crates.
  • The provenance comment for the vendored fixture changes from commit ef3e6d4d (sha256 a1f24b61…226df) to protocol 1.1.0 (sha256 9b185585…e7bc).

Tests

  • Macro unit test: A new test runs syn::parse_str::<MacroArgs> on a full attribute string. It checks that the error names the reservation and that the reserved names are still accepted as operations.
  • Runtime unit tests:
    • namespace_rejects_reserved_ns_and_nsapi
    • reservation_is_exact_match_and_namespace_only

The CachekitIO server parses a key starting `ns:` or `nsapi:` as
namespace-prefixed, so an interop key in either namespace was rejected (400)
or scoped to a namespace named after the operation. interop/v1 fixture 1.1.0
reserves both as namespaces (exact match; operations are unaffected).

BREAKING CHANGE: interop_key and #[cachekit(namespace = "ns" | "nsapi")] now reject those namespaces (runtime InvalidKey / compile error). Such keys were already rejected or misrouted by CachekitIO, but worked on Redis, Memcached and file backends.
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: cachekit-io/cachekit-rs/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 67491c47-aafc-497f-882f-c0772f1374a1

📥 Commits

Reviewing files that changed from the base of the PR and between e0d4388 and 36c1a17.

📒 Files selected for processing (6)
  • README.md
  • crates/cachekit-macros/src/lib.rs
  • crates/cachekit/src/interop.rs
  • crates/cachekit/tests/interop_vector_tests.rs
  • crates/cachekit/tests/macro_tests.rs
  • crates/cachekit/tests/vectors/interop-mode.json

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The change reserves the exact namespace values ns and nsapi in runtime and macro validation. Both values remain valid operation names. Updated interop vectors and tests cover the rules.

Changes

Reserved namespace validation

Layer / File(s) Summary
Runtime validation and interop vectors
crates/cachekit/src/interop.rs, crates/cachekit/tests/interop_vector_tests.rs, crates/cachekit/tests/vectors/interop-mode.json
interop_key rejects ns and nsapi as namespaces. Operations may use either value, and near-match namespaces remain valid. The interop vectors and vector-count assertions cover these cases.
Compile-time validation and macro tests
crates/cachekit-macros/src/lib.rs, crates/cachekit/tests/macro_tests.rs, README.md
parse_segment rejects the exact reserved namespace values at compile time and permits them as operation values. The tests use app for namespaced keys, and the documentation describes the reservation.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 36c1a

The reserved-namespace behavior is reflected consistently in the supplied runtime, macro, test, and documentation summaries. No actionable issue remains before merge.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 36c1a

The restriction prevents keys with reserved prefixes from being generated, but it is a breaking change for deployments that use those namespaces. Renaming them causes cache misses, and the impact of mixed-version deployment or rollback is not established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The new rejection is limited to exact ns and nsapi namespace values. Operation values and near-match namespaces remain accepted, limiting the changed key-generation surface.

Trust Boundaries and Controls

  • observed — Runtime input is rejected before key construction, and macro attributes are checked at parse time. A separate client namespace prefix is already rejected for interop reads rather than silently rewriting the storage key.

Resilience and Maintainability Implications

  • inferred — Same-key cache recovery does not bridge old and renamed namespaces. The operational effect is a cold cache for affected deployments, not an evidenced bypass of the new validation.

Hardening Proposals

  • proposed — Confirm the protocol update is available before release, and give affected deployments a namespace-rename and rollback plan that accounts for cold-cache load and old-key expiry without relying on ambiguous reserved-prefix keys.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarises the main change: rejecting the reserved ns and nsapi namespaces in interop mode. It is concise and specific.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 4 files. (2 skipped: 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

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

@kodus-27b

kodus-27b Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Providing Context (Files & MCPs)

Add these hints in your PR description (or a comment) to unlock deeper checks:

  • Ticket / Acceptance Criteria: `Refs: ABC-123` (Linear/Jira/Asana/ClickUp/Trello) or a direct ticket link.
  • Bugfix Validation: a Sentry/Datadog/Bugsnag event link (or paste the stack trace/error message).
  • Endpoint Risk: mention the route (e.g., `POST /api/payments`) or controller/action name.
  • Attach a repo file as context: use an explicit marker like `@file:docs/guide.mdx#L10-L50` (replace with your real path).
  • API Contract Docs: include `@file:openapi.yaml` or `@file:swagger.json` when changing routes/schemas.
  • Definition of Done / Standards: include `@file:DOD.md` or `@file:CONTRIBUTING.md` if your repo has them.
  • Design System Source of Truth: include `@file:ui/index.ts` (replace with your DS entrypoint path).
  • Feature Flags: include the flag key/name and `@file:flags.ts` / `@file:config.json` (and optionally the PostHog flag name).
  • Edge/CDN Rules: link the Cloudflare rule/zone or describe the intended redirect/header behavior.
  • Attach an MCP tool output: use `@mcp<provider|tool>` (replace with an installed MCP provider + tool, e.g., `@mcp<sentry|events.search>`).
Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ✅

Access your configuration settings here.

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