Skip to content

security: send lock_id via X-CacheKit-Lock-Id header, not query string (#63)#65

Merged
27Bslash6 merged 1 commit into
mainfrom
fix/63-lock-id-header
Jun 11, 2026
Merged

security: send lock_id via X-CacheKit-Lock-Id header, not query string (#63)#65
27Bslash6 merged 1 commit into
mainfrom
fix/63-lock-id-header

Conversation

@27Bslash6

@27Bslash6 27Bslash6 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Addresses the lock_id-header item of #63.

releaseLock sent the lock capability token as ?lock_id= on the unlock DELETE. Query strings leak into access/proxy logs and OpenTelemetry http.url spans (CWE-532), allowing replay within the lock TTL.

Change

  • LockableCachekitIO.releaseLock now sends the token in the X-CacheKit-Lock-Id header and drops it from the URL.
  • CachekitIOCore.requestJson gains an optional headers arg (backward-compatible; forwards to the existing private request, which already supports per-call headers).

Wire compatibility

SaaS dual-reads the header + legacy ?lock_id= (prefers header), shipped in saas#140 / deployed in 0.1.7. Wire-compatible with live api.cachekit.io. Ratified in protocol spec/saas-api.md.

Tests

Added an assertion that the token rides the header and never appears in the URL (no query smuggling / log leak). 20/20 lockable tests pass; tsc --noEmit + eslint clean.

Out of scope

The separate core 0.2.1 NAPI rebuild item of #63 is not in this PR (it's now actionable since cachekit-core 0.2.1 published — tracked separately in #63).

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Enhanced security by moving lock tokens from query strings to request headers, preventing potential token exposure in logs and telemetry.
  • Tests

    • Added test coverage for secure lock token transmission via request headers.

#63)

releaseLock sent the lock capability token as ?lock_id= on the unlock DELETE.
Query strings leak into access/proxy logs and OpenTelemetry http.url spans
(CWE-532), allowing token replay within the lock TTL.

Move the token to the X-CacheKit-Lock-Id request header (requestJson gains an
optional headers arg) and drop it from the URL. SaaS dual-reads header + legacy
?lock_id= (deployed in saas 0.1.7), so this is wire-compatible with prod.
Ratified in protocol spec/saas-api.md.

Scope: the lock_id-header item of #63 only. The separate core 0.2.1 NAPI
rebuild item remains tracked in #63.
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3ee57905-0c80-41b7-976b-dd140c65ee3f

📥 Commits

Reviewing files that changed from the base of the PR and between 953321b and 1a1ffc2.

📒 Files selected for processing (3)
  • packages/cachekit/src/backends/cachekitio-lockable.test.ts
  • packages/cachekit/src/backends/cachekitio-lockable.ts
  • packages/cachekit/src/backends/cachekitio.ts

Walkthrough

The PR hardens lock token security by moving authentication tokens from URL query parameters to request headers, preventing accidental exposure in logs and telemetry. It extends the request helper to accept optional headers and implements lock release via the X-CacheKit-Lock-Id header with test coverage.

Changes

Lock token header security hardening

Layer / File(s) Summary
Request helper header support
packages/cachekit/src/backends/cachekitio.ts
requestJson now accepts an optional headers parameter that is forwarded to the underlying request call, enabling custom header transmission while conditionally setting Content-Type only when a request body is present.
Lock release security implementation
packages/cachekit/src/backends/cachekitio-lockable.ts, packages/cachekit/src/backends/cachekitio-lockable.test.ts
Introduces LOCK_ID_HEADER constant with protocol documentation, updates releaseLock to transmit the lock token via the X-CacheKit-Lock-Id request header instead of query parameters, and adds test coverage verifying header transmission and token exclusion from the URL.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

🚥 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 and concisely describes the main security-focused change: sending lock_id via header instead of query string, which is the primary objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/63-lock-id-header

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install timed out. The project may have too many dependencies for the sandbox.


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

@27Bslash6 27Bslash6 merged commit 40df857 into main Jun 11, 2026
9 checks passed
@27Bslash6 27Bslash6 deleted the fix/63-lock-id-header branch June 11, 2026 11:49
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