:security: cors implemented #59
Conversation
WalkthroughThe recent update involves refining the CORS (Cross-Origin Resource Sharing) configuration in a server application by removing the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- crates/libs/cerium/src/env.rs (3 hunks)
- crates/libs/cerium/src/server/mod.rs (3 hunks)
Additional comments: 2
crates/libs/cerium/src/env.rs (2)
- 13-13: The addition of the
cors_allowed_originfield to theEnvironmentstruct is a significant change. It's crucial to ensure that the typeVec<HeaderValue>is appropriate for storing CORS origins and that it aligns with how these values are used elsewhere in the application.- 1-7: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [4-13]
The removal of the
Deserializetrait from theEnvironmentstruct indicates a shift in how environment configurations are handled. Ensure that all configurations previously managed through deserialization are now correctly handled through the environment variables and that this change does not negatively impact the application's configuration flexibility or robustness.
crates/libs/cerium/src/env.rs
Outdated
| cors_allowed_origin: env::var("ALLOWED_ORIGINS").unwrap_or("".to_string()).split(',') | ||
| .map(|origin| origin.parse::<HeaderValue>().unwrap()).collect() |
There was a problem hiding this comment.
The parsing of the ALLOWED_ORIGINS environment variable into cors_allowed_origin uses unwrap() which could cause the application to panic if the environment variable is not set or if any of the origins cannot be parsed into a HeaderValue. Consider adding error handling or a default value to improve the robustness of this implementation.
- .map(|origin| origin.parse::<HeaderValue>().unwrap()).collect()
+ .map(|origin| origin.parse::<HeaderValue>().expect("Invalid ALLOWED_ORIGINS format")).collect()Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| cors_allowed_origin: env::var("ALLOWED_ORIGINS").unwrap_or("".to_string()).split(',') | |
| .map(|origin| origin.parse::<HeaderValue>().unwrap()).collect() | |
| cors_allowed_origin: env::var("ALLOWED_ORIGINS").unwrap_or("".to_string()).split(',') | |
| .map(|origin| origin.parse::<HeaderValue>().expect("Invalid ALLOWED_ORIGINS format")).collect() |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (6)
- .cargo/config.example.toml (1 hunks)
- .cargo/config.local.toml (1 hunks)
- crates/libs/cerium/src/client/mod.rs (2 hunks)
- crates/libs/cerium/src/env.rs (2 hunks)
- crates/libs/cerium/src/server/mod.rs (3 hunks)
- crates/services/api/src/main.rs (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- crates/libs/cerium/src/env.rs
- crates/libs/cerium/src/server/mod.rs
Additional comments: 3
crates/services/api/src/main.rs (2)
- 28-28: The update to initialize
Clientwith fewer arguments aligns with the changes in theClientstruct. This simplification is a good practice, making the code cleaner and more maintainable.- 44-44: Awaiting the
set_routermethod call is correct and aligns with asynchronous programming practices in Rust. This ensures that the router setup completes before the application starts running.crates/libs/cerium/src/client/mod.rs (1)
- 20-26: The update to the
Clientstruct and thenewmethod to include and initialize theenvfield with an optionalEnvironmentparameter is a good practice. It enhances configurability and aligns with the objective of managing CORS policies more effectively.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- .cargo/config.example.toml (1 hunks)
- crates/libs/cerium/src/server/mod.rs (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- .cargo/config.example.toml
- crates/libs/cerium/src/server/mod.rs
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| &std::env::var("STORAGE_ACCESS_KEY").expect("STORAGE_ACCESS_KEY must be set."), | ||
| &std::env::var("STORAGE_ACCESS_SECRET").expect("STORAGE_ACCESS_SECRET must be set."), | ||
| &std::env::var("STORAGE_BASE_URL").expect("STORAGE_BASE_URL must be set."), | ||
| &*environment.storage_access_key.clone(), |
Check warning
Code scanning / clippy
deref which would be done by auto-deref Warning
| &std::env::var("STORAGE_ACCESS_SECRET").expect("STORAGE_ACCESS_SECRET must be set."), | ||
| &std::env::var("STORAGE_BASE_URL").expect("STORAGE_BASE_URL must be set."), | ||
| &*environment.storage_access_key.clone(), | ||
| &*environment.storage_access_secret.clone(), |
Check warning
Code scanning / clippy
deref which would be done by auto-deref Warning
| &std::env::var("STORAGE_BASE_URL").expect("STORAGE_BASE_URL must be set."), | ||
| &*environment.storage_access_key.clone(), | ||
| &*environment.storage_access_secret.clone(), | ||
| &*environment.storage_base_url.clone() |
Check warning
Code scanning / clippy
deref which would be done by auto-deref Warning
|
/prbot describe |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- .cargo/config.example.toml (1 hunks)
- .cargo/config.local.toml (1 hunks)
- crates/libs/cerium/src/env.rs (2 hunks)
- crates/libs/cerium/src/server/mod.rs (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- .cargo/config.example.toml
- .cargo/config.local.toml
- crates/libs/cerium/src/env.rs
- crates/libs/cerium/src/server/mod.rs
Summary by CodeRabbit
New Features
Refactor
Clientstruct to include a new fieldenvof typeEnvironmentfor better environmental configuration handling.Environmentstruct to include acors_allowed_originfield for parsing and storing CORS origins.Bug Fixes
get_configfunction inmain.rsto initializeClientwith corrected arguments.Environment::default().cors_allowed_origin.