🐛 bug: prevent SharedState namespace key collisions by gaby · Pull Request #4274 · gofiber/fiber

gaby · 2026-05-11T00:17:22Z

Motivation

A recent change stopped hex-encoding caller-supplied keys before concatenating them with the SharedState prefix, making namespace prefixes and keys ambiguous and enabling cross-namespace collisions on shared backends.
Restore an unambiguous storage-key derivation to prevent a crafted key in one app from reading/writing/deleting state from another app sharing the same backend.

Description

Restore hex-encoding of the caller key in SharedState.storageKey, now returning utils.CopyString(s.prefix + hex.EncodeToString(utils.UnsafeBytes(key))) to preserve a clear namespace delimiter.
Add TestSharedState_KeyNamespacingWithOverlappingPrefixes that reproduces the overlapping-prefix scenario (app vs app-one) and verifies namespaces remain isolated.
Update generated interface comment text via make generate which updated ctx_interface_gen.go and req_interface_gen.go as part of the repository generation workflow.

coderabbitai · 2026-05-11T00:17:36Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 982136f0-7c02-4479-83a8-6fe84fb317ae

📥 Commits

Reviewing files that changed from the base of the PR and between 13ad7e3 and 746878c.

📒 Files selected for processing (1)

shared_state.go

🚧 Files skipped from review as they are similar to previous changes (1)

shared_state.go

Walkthrough

Clarifies IsFromLocal() docs to state loopback-IP detection and changes SharedState.storageKey to hex-encode user keys before prefixing; adds a test ensuring namespacing prevents collisions for overlapping app-name prefixes.

Changes

Interface Documentation Updates

Layer / File(s)	Summary
IsFromLocal() Documentation Clarification `ctx_interface_gen.go`, `req_interface_gen.go`	Documentation comments for `IsFromLocal()` are updated in both Ctx and Request interfaces to specify that the method returns true when the request originates from a loopback IP, replacing the less precise "local" terminology.

Shared State Key Namespacing Fix

Layer / File(s)	Summary
Hex-Encoded Key Storage Implementation `shared_state.go`	The `storageKey` method now returns `s.prefix + hex.EncodeToString(utils.UnsafeBytes(key))` (wrapped with `utils.CopyString`); `encoding/hex` import added.
Namespace Collision Prevention Test `shared_state_test.go`	Adds `TestSharedState_KeyNamespacingWithOverlappingPrefixes` to assert hex-encoding prevents key collisions when app-name prefixes overlap (e.g., `"app"` vs `"app-one"`).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

gofiber/fiber#4270: Updates IsFromLocal() interface documentation in the same generated interface files and adds related helpers.
gofiber/fiber#4243: Modifies SharedState implementation and key namespacing, related to storage key encoding changes.

Suggested reviewers

sixcolors
efectn
ReneWerner87

Poem

🐰 I hex my keys so prefixes play nice,
loopback whispers now called precise.
Tests hop in, ensure no clash,
little bytes stay clean and brash.
Hooray — the storage race, at peace! 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description provides motivation, details the key changes, and explains the test additions. However, it lacks structured sections matching the template (checklist items, type of change selection, and documentation updates are not addressed).	Complete the description by selecting appropriate change type checkbox, confirming unit tests pass, and noting any documentation updates made or needed.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main bug fix: preventing namespace key collisions in SharedState, which is the primary focus of the changeset.
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-sharedstate-raw-key-vulnerability

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.

🔧 golangci-lint (2.12.1)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

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 and usage tips.}

gemini-code-assist

Code Review

This pull request updates the documentation for IsFromLocal to clarify loopback IP usage and modifies the SharedState key generation to use hex encoding, preventing potential key collisions between applications with overlapping prefixes. A new test case validates this namespacing logic. Feedback suggests removing a redundant utils.CopyString call in storageKey to avoid unnecessary allocations, as string concatenation and hex encoding already produce fresh strings.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Copilot

Pull request overview

This PR restores unambiguous SharedState storage-key derivation to prevent cross-namespace key collisions when multiple Fiber apps share the same backend storage.

Changes:

Reintroduce hex-encoding of caller-supplied SharedState keys when deriving the backend storage key.
Add a regression test covering overlapping default prefixes (e.g., app vs app-one) to ensure isolation.
Regenerate interface files to update IsFromLocal comment wording.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
shared_state.go	Restores hex-encoding when deriving the storage key to prevent prefix/key ambiguity collisions.
shared_state_test.go	Adds a regression test validating isolation between overlapping default prefixes.
req_interface_gen.go	Regenerated comment text for `IsFromLocal` to clarify loopback semantics.
ctx_interface_gen.go	Regenerated comment text for `IsFromLocal` to clarify loopback semantics.

codecov · 2026-05-11T00:22:17Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.21%. Comparing base (c48bbc2) to head (746878c).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4274   +/-   ##
=======================================
  Coverage   91.21%   91.21%           
=======================================
  Files         130      130           
  Lines       12760    12760           
=======================================
  Hits        11639    11639           
  Misses        709      709           
  Partials      412      412

Flag	Coverage Δ
unittests	`91.21% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gaby · 2026-05-11T00:22:36Z

@copilot Error: shared_state.go:416:1: File is not properly formatted (gofmt)

Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/60c247c0-fd61-493b-8cda-5337dbb8023f Co-authored-by: gaby <835733+gaby@users.noreply.github.com>

Copilot · 2026-05-11T00:29:51Z

@copilot Error: shared_state.go:416:1: File is not properly formatted (gofmt)

Fixed in 746878c by applying gofmt to shared_state.go; the lint/format checks now pass locally.

🐛 bug: prevent SharedState namespace key collisions

4997853

Copilot AI review requested due to automatic review settings May 11, 2026 00:17

gaby requested a review from a team as a code owner May 11, 2026 00:17

gaby added codex aardvark labels May 11, 2026 — with ChatGPT Codex Connector

gaby requested review from ReneWerner87, efectn and sixcolors May 11, 2026 00:17

ReneWerner87 added v3 ☢️ Bug labels May 11, 2026

ReneWerner87 added this to v3 May 11, 2026

ReneWerner87 added this to the v3 milestone May 11, 2026

Copilot started reviewing on behalf of gaby May 11, 2026 00:18 View session

gemini-code-assist Bot reviewed May 11, 2026

View reviewed changes

Comment thread shared_state.go Outdated

Update shared_state.go

13ad7e3

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Copilot AI reviewed May 11, 2026

View reviewed changes

Comment thread shared_state.go Outdated

Copilot started work on behalf of gaby May 11, 2026 00:22 View session

chore: format shared_state return line

746878c

Agent-Logs-Url: https://github.com/gofiber/fiber/sessions/60c247c0-fd61-493b-8cda-5337dbb8023f Co-authored-by: gaby <835733+gaby@users.noreply.github.com>

Copilot finished work on behalf of gaby May 11, 2026 00:31

ReneWerner87 removed the aardvark label May 11, 2026

ReneWerner87 approved these changes May 11, 2026

View reviewed changes

ReneWerner87 merged commit dec7195 into main May 11, 2026
21 checks passed

ReneWerner87 deleted the fix-sharedstate-raw-key-vulnerability branch May 11, 2026 06:46

github-project-automation Bot moved this to Done in v3 May 11, 2026

ReneWerner87 modified the milestones: v3, v3.3.0 May 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

🐛 bug: prevent SharedState namespace key collisions#4274

🐛 bug: prevent SharedState namespace key collisions#4274
ReneWerner87 merged 3 commits into
mainfrom
fix-sharedstate-raw-key-vulnerability

gaby commented May 11, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 11, 2026 •

edited

Loading

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

codecov Bot commented May 11, 2026 •

edited

Loading

Uh oh!

gaby commented May 11, 2026

Uh oh!

Copilot AI commented May 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

gaby commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Description

Uh oh!

coderabbitai Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codecov Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gaby commented May 11, 2026

Uh oh!

Copilot AI commented May 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gaby commented May 11, 2026 •

edited

Loading

coderabbitai Bot commented May 11, 2026 •

edited

Loading

codecov Bot commented May 11, 2026 •

edited

Loading