Skip to content

Validate storage region to prevent hostname injection#15

Merged
ractive merged 1 commit into
mainfrom
security/audit-fixes
Mar 19, 2026
Merged

Validate storage region to prevent hostname injection#15
ractive merged 1 commit into
mainfrom
security/audit-fixes

Conversation

@ractive

@ractive ractive commented Mar 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • CRITICAL fix: StorageClient::new() now validates --region against a whitelist of known Bunny CDN storage regions (storage, uk, ny, la, sg, syd, br, jh, se)
  • Previously, a malicious --region evil.com value would construct https://evil.com.bunnycdn.com and send the AccessKey header to an attacker-controlled host
  • Adds 3 unit tests covering valid regions, unknown regions, and empty regions

Test plan

  • cargo fmt — clean
  • cargo clippy -- -D warnings — clean
  • cargo test — all tests pass
  • Manual: verify hoppy storage ls --zone test --region evil.com now returns an error
  • Manual: verify hoppy storage ls --zone test --region la still works

🤖 Generated with Claude Code

StorageClient::new() now rejects unknown --region values against a
whitelist of known Bunny CDN regions. Previously, an attacker-controlled
region like "evil.com" would produce https://evil.com.bunnycdn.com and
send the AccessKey header to an attacker-controlled host.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Edge Storage client construction path by rejecting unexpected/unsafe region values, preventing caller-controlled input from influencing the request hostname in a way that could leak the AccessKey header.

Changes:

  • Add a VALID_REGIONS allowlist and validate region in bunny-api-storage::StorageClient::new.
  • Make StorageClient::new fallible (Result<Self>) and update CLI call sites to propagate errors.
  • Add unit tests for valid, unknown, and empty region inputs.

Reviewed changes

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

File Description
src/commands/storage.rs Propagates the now-fallible StorageClient::new(..)? and keeps debug/record configuration behavior intact.
crates/bunny-api-storage/src/client.rs Introduces region allowlist validation in the storage client constructor and adds targeted unit tests.

Comment on lines +75 to +76
pub fn new(region: &str, access_key: impl Into<String>) -> Result<Self> {
if !VALID_REGIONS.contains(&region) {

Copilot AI Mar 19, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StorageClient::new now returns Result<Self> instead of Self, which is a breaking public API change for bunny-api-storage consumers. If this crate is published/semver-tracked, consider either (a) bumping the crate version accordingly, or (b) keeping the old new signature (e.g., delegating to a fallible try_new) to preserve source compatibility while still preventing hostname injection.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This crate is 0.1.0, unpublished, and only consumed within the workspace — the breaking change is intentional and all call sites are updated in this PR. No version bump needed.

@ractive
ractive merged commit 434ed52 into main Mar 19, 2026
9 of 11 checks passed
@ractive
ractive deleted the security/audit-fixes branch March 19, 2026 23:54
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.

2 participants