Skip to content

fix(kms): auto-append /prpc to onboard source_url #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kvinwang wants to merge 1 commit into from
Closed

fix(kms): auto-append /prpc to onboard source_url #8

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion kms/src/onboard_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,13 @@ impl OnboardRpc for OnboardHandler {
}

async fn onboard(self, request: OnboardRequest) -> Result<OnboardResponse> {
let source_url = if request.source_url.ends_with("/prpc") {
request.source_url.clone()
} else {
format!("{}/prpc", request.source_url.trim_end_matches('/'))
Comment on lines +80 to +83
Copy link

@sentry sentry bot Feb 13, 2026

Choose a reason for hiding this comment

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

Bug: URL normalization incorrectly appends a duplicate /prpc suffix if the input URL already ends with /prpc/, causing downstream requests to fail.
Severity: MEDIUM

Suggested Fix

The normalization logic should be adjusted to correctly handle URLs with a trailing slash. A robust approach is to first trim any trailing slashes from request.source_url, and then check if the resulting string ends with /prpc before conditionally appending it. This ensures that inputs like .../prpc/ are correctly normalized to .../prpc.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: kms/src/onboard_service.rs#L80-L83

Potential issue: The URL normalization logic in `onboard_service.rs` incorrectly handles
URLs that end with `/prpc/`. The check `request.source_url.ends_with("/prpc")` returns
false for such an input due to the trailing slash. This causes the code to enter the
`else` block, which trims the trailing slash and then appends another `/prpc` suffix.
This results in a malformed URL like `https://host/prpc/prpc`. Subsequent RPC calls
using this URL will fail with a 404 error, breaking the functionality for clients that
provide URLs with a trailing slash, such as direct API callers or configurations.

Did we get this right? 👍 / 👎 to inform future reviews.

};
Comment on lines +80 to +84
Copy link

@sentry sentry bot Feb 13, 2026

Choose a reason for hiding this comment

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

Bug: The URL normalization logic incorrectly handles a source_url ending with /prpc/, resulting in a malformed URL like .../prpc/prpc.
Severity: MEDIUM

Suggested Fix

Reverse the order of operations in the URL normalization logic. First, call trim_end_matches('/') on the source_url to remove any trailing slashes. Then, perform the ends_with("/prpc") check to decide whether to append /prpc. This ensures that inputs like https://host/prpc/ are correctly handled.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: kms/src/onboard_service.rs#L80-L84

Potential issue: The URL normalization logic has a flaw in its order of operations. When
a `source_url` ending in `/prpc/` (with a trailing slash) is provided, the code first
checks if the string `ends_with("/prpc")`. This check returns false due to the trailing
slash. Consequently, the code enters the `else` block, where it trims the trailing slash
and then appends another `/prpc`. This results in a malformed URL, such as
`https://host/prpc/prpc`, which will cause subsequent API calls to fail with 404 errors,
breaking the onboarding functionality for that user.

Did we get this right? 👍 / 👎 to inform future reviews.

let keys = Keys::onboard(
&request.source_url,
&source_url,
&request.domain,
self.state.config.onboard.quote_enabled,
self.state.config.pccs_url.clone(),
Expand Down