Skip to content

fix: harden multi-cloud routing and Azure/GCP service correctness - #195

Open
skyoo2003 wants to merge 1 commit into
mainfrom
fix/multicloud-review-findings
Open

skyoo2003 wants to merge 1 commit into
mainfrom
fix/multicloud-review-findings

Conversation

@skyoo2003

Copy link
Copy Markdown
Owner

Summary

Fixes the correctness, routing, and security bugs that two code-review passes found in #194: multi-cloud gateway routing, the Azure Key Vault, Azure App Configuration, GCP Secret Manager and GCP Pub/Sub providers, and the workload runtime.

Related Issue

Refs #194

Changes

Gateway routing (internal/gateway/protocol.go)

  • A new azureVaultService routes *.vault.azure.net and *.azconfig.io hosts directly. A bare /secrets, /keys or /kv path routes to Azure only if the request also has api-version. Before this, S3 buckets and Azure blob containers whose names started with those prefixes were hijacked.
  • Azure vault detection now runs before the storage checks, so a Key Vault secret named messages-* no longer goes to Queue.
  • GCP routing matches the collection path segment instead of a substring. A Pub/Sub project named secrets-* no longer goes to Secret Manager.

Workload / Lambda

  • oci.go: docker is the first auto-detected runtime again. Error messages name only the subcommand, so --env K=V secrets no longer leak into 503 bodies.
  • Lambda: the API and event-source-mapping prefixes now require the exact path or the path plus /, so /2015-03-31/functionsX no longer matches.

GCP Secret Manager

  • Added the missing GetSecretVersion handler.
  • Versions are copied under the lock in GetSecretVersion, AddSecretVersion and ListSecretVersions, which fixes a data race with destroy.
  • latest now means the newest version created, matching GCP.
  • secretId is validated against ^[A-Za-z0-9_-]{1,255}$.
  • Removed an unused data directory that was created on startup.

GCP Pub/Sub

  • Messages not acked before their ack deadline are delivered again.
  • CreateSubscription returns 404 if the topic does not exist.

Azure Key Vault

  • A SetSecret with no value returns 400.
  • List responses return an empty array, never null.
  • Listing versions of an unknown secret returns 404.
  • ListResources now lists every vault.

Azure App Configuration

  • label=%00 matches unlabeled keys.
  • Keys are decoded once instead of twice.
  • ListResources now lists every store.

Docs

  • docs/configuration.md: the YAML example no longer sets both the top-level services block and providers.aws.services. With both set, sqs and dynamodb did not start.

Test Plan

  • Added regression tests for gateway routing, Pub/Sub redelivery and missing topics, the Secret Manager version lifecycle and latest semantics, and secretId validation.
  • Not run yet. No Go toolchain was available where these changes were made. CI is the first build.

Known issues before merge

  • Key Vault SetSecret 400 likely breaks the Azure SDK. The SDK sends a bodyless probe first, and the emulator never answers with a 401 WWW-Authenticate challenge. Either revert to accepting an empty body or add the challenge and update the compat tests.
  • Codegen drift. The new GetSecretVersion case needs make codegen and updated figures in docs/coverage.md.

Checklist

  • Self-reviewed the code
  • Added/updated tests
  • Lint/format passes (golangci-lint run)
  • Updated documentation (if applicable)
  • Added a Changie changelog fragment for user-facing changes (changie new, one sentence and two at most, see RELEASE.md) — or N/A (docs/tests/chore only)

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests Test code and test infrastructure services AWS service implementations labels Oct 13, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation services AWS service implementations tests Test code and test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant