Commit Graph

6 Commits

Author SHA1 Message Date
smattymatty 54c63387cb fix(cors): include Access-Control-Allow-Headers in permissive OPTIONS placeholder (#1450)
The OPTIONS placeholder for buckets without a resolvable global alias returns` Access-Control-Allow-Origin: *` and `Access-Control-Allow-Methods: *` but omits `Access-Control-Allow-Headers`.

Bug verified against Garage v2.2.0 with a local-aliased bucket: OPTIONS placeholder doesn't have `Access-Control-Allow-Headers`, causes the browser to reject signed PUT preflights

The current placeholder fails open for unsigned simple requests but blocks every signed request, undermining the design intent flagged in the FIXME:

```rs
// We take the permissive approach of allowing everything,
// because we don't want to prevent web apps that use
// local bucket names from making API calls.
```

Adds `Access-Control-Allow-Headers: *` so the permissive default is actually permissive for the request shapes that exist in practice.

Refs #258. Does not address the broader FIXME (CORS rule resolution for local-aliased buckets); the placeholder approach is preserved.

All tests are fine locally:

```bash
 ▲ ~/opensource/garage cargo test -p garage_api_common cors::

running 5 tests
test cors::tests::preflight_with_single_allowed_origin_returns_request_origin ... ok
test cors::tests::preflight_with_multiple_allowed_origins_reflects_request_origin ... ok
test cors::tests::preflight_with_wildcard_allowed_origin_returns_wildcard ... ok
test xml::cors::tests::test_deserialize_norules ... ok
test xml::cors::tests::test_deserialize ... ok

test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 16 filtered out; finished in 0.00s
```

Co-authored-by: smattymatty <smattymatt@gmail.com>
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1450
Reviewed-by: Alex <lx@deuxfleurs.fr>
2026-05-12 08:17:48 +00:00
Minkyu Kim a2c797000f fix(cors): return single matching origin instead of multiple values in Access-Control-Allow-Origin (#1419)
## Title
fix(cors): return single matching origin instead of multiple values in `Access-Control-Allow-Origin`

## Summary
This PR fixes bucket CORS responses when a single CORS rule contains multiple `AllowedOrigins`.

Previously, Garage returned the configured origins as a comma-separated list in `Access-Control-Allow-Origin`, for example:

```http
Access-Control-Allow-Origin: https://app.example.test, https://admin.example.test
```

This is not the expected browser-facing behavior.
When a request origin matches a configured rule, the response should reflect **only the matching request origin**, unless the rule contains `*`.

## What changed
- `Access-Control-Allow-Origin` now behaves as follows:
  - returns `*` when the matched rule contains a wildcard origin
  - otherwise returns the request `Origin` as a **single value**
- added `Vary: Origin` when ACAO reflects the request origin
- added preflight-specific `Vary` handling in the preflight path for:
  - `Origin`
  - `Access-Control-Request-Method`
  - `Access-Control-Request-Headers`

## Scope
This change applies to shared bucket CORS handling paths, including:
- S3 API responses
- K2V API responses
- S3 POST object responses
- web bucket responses
- preflight (`OPTIONS`) bucket CORS responses

This does **not** change admin API fixed CORS behavior.

## Reproduction
A direct repro script is included:

```bash
./script/test-cors-multi-origin.sh
```

It exercises two cases against a direct single-node Garage instance:

1. **single-origin control**
2. **multi-origin repro**

Before this fix, the multi-origin case returned a comma-separated ACAO value.

After this fix, both cases reflect only the request origin.

## Example behavior

### Before
```http
Access-Control-Allow-Origin: https://app.example.test, https://admin.example.test
```

### After
```http
Access-Control-Allow-Origin: https://app.example.test
```

## Tests
Added/updated tests in `src/api/common/cors.rs` for:
- single-origin control
- multiple allowed origins reflecting the request origin
- wildcard origin preserving `*`
- preserving existing `Vary` values while appending `Origin`

## Validation
Used for validation:

```bash
cargo test -p garage_api_common cors::tests -- --nocapture
cargo build -p garage --bin garage
./script/test-cors-multi-origin.sh
```

## Reproducibility
For reviewers who want to validate behavior by commit:

- Before fix: `aa368e4b`
  - includes the direct repro script and the regression test setup
  - multi-origin ACAO is reproduced as a comma-separated value

- After fix: `f630eb92`
  - reflects only the matching request origin
  - preserves wildcard behavior
  - adds `Vary: Origin` and preflight-specific `Vary` handling

Branch:
- `fix/cors-multiple-allow-origin`

Base used during validation:
- `74ad3bf8` (`main-v2`)

Closes Deuxfleurs/garage#1149

Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1419
2026-04-28 14:48:02 +00:00
Gwen Lg e331f88c85 chore: fix typos in rust code comment or error message 2026-01-29 14:53:27 +01:00
Alex Auvolat 1e13a66b42 rework bucket helper functions to use local access where relevant 2025-03-25 13:00:48 +01:00
Alex Auvolat c5df820e2c api: start refactor of signature to calculate checksums earlier 2025-02-17 18:47:06 +01:00
Alex Auvolat afa28706e5 split s3/cors.rs into also common/cors.rs 2025-01-31 18:42:14 +01:00