Commit Graph

138 Commits

Author SHA1 Message Date
trinity-1686a da568cf56f fix reflected xss when returning errors on web endpoint
Reported by Filippo Decortes from ares-cyber.ai
2026-06-10 19:44:27 +02:00
Alex Auvolat bf0a24ea69 replace Option CRDT by explicit CancelingOption and MergingOption types 2026-05-13 11:20:06 +02: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
Alex Auvolat 7b119c0b4f bump version number to v2.3.0 2026-04-16 18:34:27 +02:00
Gwen Lg f59a8b7f62 style: corrects the use of ';' to improve readability
- remove unnecessary semicolon
and enable lint warning: unnecessary semicolon
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#unnecessary_semicolon
- add `;` to the last statement for consitent formatting
and enable lint `clippy::semicolon_if_nothing_returned`
warning: consider adding a `;` to the last statement for consistent formatting
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#semicolon_if_nothing_returned
2026-02-17 18:38:44 +00:00
Gwen Lg 6cde00073f chore: enable workspace configuration of lints in Cargo.toml
and use workspace configuration in each package.
This allow to customize clippy and rust lint configuration for project.
No particular configuration in this commit.
2026-02-17 18:38:44 +00:00
Gwen Lg 4a0be692b3 docs: various fixes in Rust code documentation
- add backticks on struct doc comments
lint message: unclosed HTML tag `M`
- add a blanck line for proper doc formating
lint message: doc list item without indentation
help: if this is supposed to be its own paragraph, add a blank line
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#doc_lazy_continuation
- fix hyperlink in doc + one url invalid
lint message: this URL is not a hyperlink
note: bare URLs are not automatically turned into clickable links
- ajust space in doc
lint message: doc list item overindented
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#doc_overindented_list_items
2026-01-29 20:49:28 +01:00
Gwen Lg 21db7a4d4b chore: remove various useless code
- call expect directly on Result
lint message: called `ok().expect()` on a `Result` value
help: you can call `expect()` directly on the `Result`
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#ok_expect
- use assign operation instead of manual implementation
lint message: manual implementation of an assign operation
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#assign_op_pattern
- remove useless call to format
lint message: useless use of `format!`
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#useless_format
- remove useless `?Sized`
lint message: `?Sized` bound is ignored because of a `Sized` requirement
note: ...because `Deserialize` has the bound `Sized`
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#needless_maybe_sized
- use is_some instead of pattern maching with Some(_)
lint message: redundant pattern matching, consider using `is_some()`
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#redundant_pattern_matching
- remove unneeded unit return type
lint message: unneeded unit return type
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#unused_unit
- remove redundant closure
lint message: redundant closure
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#redundant_closure
- use derive Default instead of manual implementation
lint message: this `impl` can be derived
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#derivable_impls
- remove unneeded `return` statement
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#needless_return
- remove empty string from println call
lint message: empty string literal in `println!`
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#println_empty_string
- remove clone() on type than implement Copy
lint message: using `clone` on type `Option<ChecksumValue>` which implements the `Copy` trait
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#clone_on_copy
- remove useless let binding
lint message: this let-binding has unit value
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#let_unit_value
- remove useless len comparison to zero, already test of empty
lint message: length comparison to zero
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#len_zero
- remove useless `as_deref` call
lint message: derefed type is same as origin
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#needless_option_as_deref
- remove useless conversion of the same type
lint message: useless conversion to the same type: `replication_mode::ReplicationFactor`
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#useless_conversion
- remove useless bool_comparison
lint message: equality checks against false can be replaced by a negation
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#bool_comparison
- remove useless to_string
lint message: unnecessary use of `to_string`
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#unnecessary_to_owned

Signed-off-by: Gwen Lg <me@gwenlg.fr>
2026-01-29 20:49:28 +01:00
Gwen Lg 141b3f24f1 chore: clean relative to references and borrows
- lint message: the borrowed expression implements the required traits
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#needless_borrows_for_generic_args
- lint message: this expression creates a reference which is immediately dereferenced by the compiler
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#needless_borrow
- lint message: you don't need to add `&` to all patterns
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#match_ref_pat
- remove useless taken reference
lint message: needlessly taken reference of left operand
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#op_ref
- use &Path instead of &PathBuf as fn parameters
lint message: writing `&PathBuf` instead of `&Path` involves a new object where a slice will do
help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#ptr_arg
2026-01-29 20:49:28 +01:00
Gwen Lg e331f88c85 chore: fix typos in rust code comment or error message 2026-01-29 14:53:27 +01:00
Alex Auvolat 582b168b6a bump version to v2.2.0 2026-01-24 12:32:22 +01:00
Alex eac3a60050 Merge pull request 'migrate to this error, garage-v2' (#1219) from thiserror-2 into main-v2
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1219
Reviewed-by: Alex <lx@deuxfleurs.fr>
2025-11-12 08:05:43 +00:00
trinity-1686a 5333285b50 don't append index document to redirect urls 2025-11-06 18:03:58 +01:00
trinity-1686a ef913843f7 fmt 2025-11-01 19:35:11 +01:00
trinity-1686a 1fe932d07f migrate to this error
it doesn't generate a bazillion warning at compile time
2025-11-01 17:25:23 +01:00
Alex Auvolat b43f309ec7 bump version to v2.1.0 2025-09-15 15:50:02 +02:00
Alex Auvolat dc1a4ffd76 Merge branch 'main' into next-v2 2025-06-13 14:01:39 +02:00
trinity-1686a 26bc807905 put web error in a basic webpage
before, it was a plain string, with an xml content type

this caused browsers to show very ugly and meaningless pages
2025-06-10 22:23:06 +02:00
Alex Auvolat d38d62f4d7 bump version to v2.0.0 2025-04-17 12:36:41 +02:00
Alex Auvolat 29ce490dd6 Merge branch 'main' into next-v2 2025-03-05 14:50:22 +01:00
Alex Auvolat 4689b10448 bump version to v1.1.0 2025-03-05 10:19:51 +01:00
Alex Auvolat bf27a3ec98 web: implement x-amz-website-redirect-location 2025-02-19 17:04:10 +01:00
Alex Auvolat c5df820e2c api: start refactor of signature to calculate checksums earlier 2025-02-17 18:47:06 +01:00
Alex Auvolat 2f55889835 add configuration option to enable/disable monitoring bucket in web metrics 2025-02-14 14:59:00 +01:00
Jonathan Davies 8b9cc5ca3f web_server.rs: Added bucket domain to observability. 2025-02-14 14:36:20 +01:00
trinity-1686a f034e834fa Merge branch 'main' into 1686a/s3-redirects 2025-02-08 16:01:11 +01:00
Alex Auvolat 4563313f87 use cargo-shear to remove many unused dependencies between crates 2025-01-31 18:47:30 +01:00
Alex Auvolat afa28706e5 split s3/cors.rs into also common/cors.rs 2025-01-31 18:42:14 +01:00
Alex Auvolat 84f1db91c4 fix things up 2025-01-31 18:34:57 +01:00
Alex Auvolat 9fa20d45be wip: split garage_api into garage_api_{common,s3,k2v,admin} 2025-01-31 18:18:29 +01:00
Alex Auvolat 2aaba39ddc refactor web_server.rs 2025-01-04 20:11:54 +01:00
Alex Auvolat 47467df83e avoid handling status_code-related logic in api/s3/get.rs 2025-01-04 19:52:14 +01:00
Alex Auvolat 22487ceddf move Redirect::compute_target to standalone function in web_server.rs 2025-01-04 18:22:42 +01:00
trinity-1686a c939d2a936 clippy 2024-12-22 15:26:06 +01:00
trinity-1686a 65e9dde8c9 add tests 2024-12-22 15:20:09 +01:00
trinity-1686a c9b733a4a6 support redirection on s3 endpoint 2024-12-14 17:46:27 +01:00
Alex Auvolat 7a143f46fc Bump to version 1.0.1 2024-09-22 14:25:32 +02:00
Alex Auvolat afad62939e [next-0.10] bump version number to 1.0 2024-03-28 15:19:44 +01:00
Alex Auvolat bbde9bc912 Merge branch 'main' into next-0.10 2024-03-04 15:56:10 +01:00
Alex 3168bb34a0 Merge pull request 'add request context helper' (#751) from yuka/garage:req-ctx into main
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/751
2024-03-04 14:51:05 +00:00
Alex Auvolat 8670140358 [rel-0.9.3] Bump version to 0.9.3 2024-03-04 14:00:55 +01:00
Yureka fb55682c66 add request context helper 2024-03-04 13:26:39 +01:00
Alex Auvolat 6a7623e90d [rel-0.9.2] Bump version to v0.9.2 2024-03-01 16:54:39 +01:00
Alex Auvolat cf2af186fc Merge branch 'main' into next-0.10 2024-02-13 11:36:28 +01:00
Alex Auvolat 02e98e2d10 [header-override-650] implement header overriding in GetObject (fix #650) 2024-02-09 15:58:46 +01:00
Alex Auvolat 5c63193d1d [dep-upgrade-202402] fix shutdown issue introduced when upgrading hyper 2024-02-08 23:43:59 +01:00
Alex Auvolat fe48d60d2b [dep-upgrade-202402] refactor http listener code 2024-02-07 14:34:40 +01:00
Alex Auvolat a22bd31920 [dep-upgrade-202402] migration to http/hyper 1.0 for k2v api 2024-02-05 19:27:12 +01:00
Alex Auvolat 0bb5b77530 [dep-upgrade-202402] wip: port to http/hyper crates v1 2024-02-05 18:49:54 +01:00
Alex Auvolat fe1af5d98b [dep-upgrade-202402] refactor dependencies: move all as workspace deps 2024-02-05 13:02:02 +01:00