fix(interop)!: forbid double-dot inside interop segments (LAB-5906) - #94
Merged
Merged
Conversation
The segment pattern admits `..` inside a segment (`a..b`, `users.v1..beta`), but the server rejects `..` anywhere in a key (cache-key-format.md, Server-Side Requirements, Traversal row). Every SDK therefore accepted such a segment and minted a key that fails with 400 on every CachekitIO request, while the same key works on Redis and file backends. Interop keys are portable, so the grammar forbids it on every backend, the same rule the ns/nsapi reservation follows. The pattern stays a plain regex without lookahead, so every SDK can apply it as written; the ban is a separate substring check. A `..` cannot span the `:` delimiter because a segment cannot start with `.`, so a per-segment check covers the whole key. Fixture 1.2.0 adds reject_double_dot_namespace (a..b), reject_double_dot_operation (x..y) and the key vector lone_dots_stay_valid (app.v1 / users.fetch.by_id): before it, no key vector had a `.` in a segment, so an SDK that rejected every `.` passed the suite. BREAKING CHANGE: an interop namespace or operation containing `..` now raises at decoration / registration time, on every backend. Rename the segment.
…5906) reject_double_dot_operation now puts the `..` at the end of the segment (`x..`): a byte loop that skips the last pair still rejects `a..b` and `x..y`, but accepts `abc..`. The namespace vector keeps the mid-segment case. lone_dots_stay_valid now uses namespace `app.`: an implementation that splits on `.` and rejects empty labels, or rejects a trailing `.`, passed every vector before, although the grammar allows a trailing `.`. The comments now give the right reason a per-segment check covers the key: the `:` delimiters separate the segments and the hash is hex, so any `..` lies inside one segment.
Contributor
|
Warning Review limit reachedNext included review available in 38 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: cachekit-io/protocol/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (6)
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. Comment |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
Kody Code Review — 1 suggested fix. 🛠️ Open Agent Prompt |
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
This was referenced Sep 30, 2026
…-5906) The Test vectors in CI cells name fixture 1.2.0 and cachekit-py#391, cachekit-rs#98 and cachekit-ts#164, all unreleased. The Python cell says fixture 1.1.0 ships in PyPI 0.20.0, which is published.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Providing Context (Files & MCPs)Add these hints in your PR description (or a comment) to unlock deeper checks:
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
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.
Summary
Interop segments (
namespaceandoperation) may no longer contain... The segment regex^[a-z0-9][a-z0-9._-]{0,63} admits.., but the CachekitIO server rejects..anywhere in a key. As a result, every SDK accepted a segment such asa..band produced keys that returned400` on CachekitIO, while the same keys worked on Redis and file backends.The regex is unchanged and still uses no lookahead. The ban is a separate substring check that applies on every backend, following the same pattern as the
ns/nsapinamespace reservation.Modified public surfaces
spec/interop-mode.md..in either segment. A lone.stays valid, including a trailing one (app.)...rule..." caveat is removed. The grammar is now described as a strict subset of what the server validator accepts.key_vectorsgoes from 34 to 35 anderror_vectorsfrom 11 to 13.test-vectors/interop-mode.json(1.1.0 → 1.2.0)reject_double_dot_namespace: namespacea..b, with..mid-segment.reject_double_dot_operation: operationx.., with..at the end. This catches implementations that skip the final character pair.lone_dots_stay_valid: namespaceapp., operationusers.fetch.by_id, args[1]. It reuses the args hash405f09…e21afromreservation_scope, so only the key string differs. It is the first key vector with a.in a segment.segment_patternis unchanged.segment_pattern_notenow states the..rule.tools/interop-reference.pyFORBIDDEN_SUBSTRING = "..".interop_key()now raisesInteropErrorfor a segment containing... The check runs per segment, after the pattern match and before the reserved-namespace check._build()emits version1.2.0and the new vectors._self_check()asserts that both new error vectors matchsegment_pattern. This ensures they test the new rule rather than the regex.tools/interop-crosscheck.mjssegmentValid()helper that combines the pattern test with!segment.includes("..").Other files
changelog.d/20260930_lab-5906.md: new fragment that includes a "Breaking for" note.Breaking change
Any interop namespace or operation containing
..now fails at decoration or registration time in every SDK, on every backend. To migrate, rename the segment; the keys it names become a full cache miss. Such keys already failed on CachekitIO before this change.Merge order
Merge this PR before the SDK PRs. Each SDK PR vendors the 1.2.0 fixture byte-for-byte and pins its sha256.
This PR updates documentation for the interop change that forbids
..inside segments (fixture 1.2.0). Only thechangelog.d/20260930_lab-5906.mdandsdk-feature-matrix.mddiffs are included here. The breaking change named in the PR title (spec, fixtures and tools) is not in the supplied patches; it is referenced only through the existing changelog entries.Changes in
sdk-feature-matrix.md("Test vectors in CI" row)ns/nsapinamespace reservation) is now recorded as shipped in PyPI 0.20.0 (cachekit-py#350). It was previously listed as unreleased...in a segment) via cachekit-py#391, unreleased.Changes in
changelog.d/20260930_lab-5906.mdPublic API impact
Summary
The interop/v1 segment pattern
^[a-z0-9][a-z0-9._-]{0,63}$admits..inside a segment (a..b,users.v1..beta). The server rejects..anywhere in a key (cache-key-format.md → Server-Side Requirements, the Traversal row). So every SDK accepted such a segment and minted a key that fails with400on every CachekitIO request, while the same key works on Redis and file backends.This PR forbids
..in either segment, on every backend: interop keys are portable, so a segment valid on one backend must be valid on all. That is the same rule thens/nsapireservation follows. The pattern itself is unchanged and stays a plain regex without lookahead, so each SDK can apply it as written; the ban is a separate substring check. The:delimiters separate the segments and the hash is hex, so a per-segment check covers the whole key.Changes
spec/interop-mode.md: Segment grammar says a segment MUST NOT contain.., with the reason; a lone.stays valid, including at the end of a segment. SDK requirement 1 names the rule. The status banner and SaaS Considerations drop the "one known exception" text, since the grammar is now a plain subset of what the server accepts. Test Vectors counts: 35 key, 13 error.test-vectors/interop-mode.json1.2.0:reject_double_dot_namespace: namespacea..b(the..mid-segment).reject_double_dot_operation: operationx..(the..at the end of the segment, which a check that skips the last pair misses).lone_dots_stay_valid: namespaceapp., operationusers.fetch.by_id. No key vector had a.in a segment before, so an SDK that rejected every., or a trailing., passed the whole suite.segment_patternis unchanged;segment_pattern_notestates the rule.tools/interop-reference.pyrejects a..segment. Its self-check asserts that both new error vectors matchsegment_pattern, so they exercise the new rule and not the pattern.tools/interop-crosscheck.mjshard-codes the rule rather than reading it from the fixture, as it does for the reserved namespaces.changelog.d/20260930_lab-5906.md: the entry, with a Breaking for line.CHANGELOG.mdis untouched.sdk-feature-matrix.md: the "Test vectors in CI" row names fixture 1.2.0 and the SDK PRs that vendor it.Breaking
An interop namespace or operation containing
..now raises at decoration / registration time in every SDK, on every backend. Rename the segment; the keys it names become a full cache miss. Before this change, such a key already failed on CachekitIO.Merge order
Merge this PR before the three SDK PRs. Each SDK PR vendors
test-vectors/interop-mode.jsonbyte-for-byte (sha256702613766d1b92bc3a337627a96b9aedc89abfeb4d9208c2bb00c9539a0a1f40) and pins that sha.Test plan
python3 tools/interop-reference.py verify(stdlib, and again withcryptography+msgpack): 35 key, 4 value, 13 error, 1 AAD, 1 encryption.node tools/interop-crosscheck.mjswith@noble/hashes@2.2.0: all vectors verified independently...check (both error vectors fail), reject any., skip the last pair, reject empty labels after splitting on., reject a trailing.. Removing the reference tool's check fails its self-check.verify.ymlcommand passes locally, includingtools/test_changelog_collect.pyand theCHANGELOG.md-untouched check.