Skip to content

feat(billing, team): add billing and team management RPC functionality#159

Merged
graycyrus merged 4 commits intotinyhumansai:mainfrom
YellowSnnowmann:feat/team-billing-core
Apr 1, 2026
Merged

feat(billing, team): add billing and team management RPC functionality#159
graycyrus merged 4 commits intotinyhumansai:mainfrom
YellowSnnowmann:feat/team-billing-core

Conversation

@YellowSnnowmann
Copy link
Copy Markdown
Contributor

@YellowSnnowmann YellowSnnowmann commented Apr 1, 2026

Summary

Adds new billing and team domains to the Rust core so billing and team-management actions are available over the shared JSON-RPC/controller registry.

  • Introduces billing operations:

    • Current plan lookup
    • Plan purchase
    • Billing portal session creation
    • Credit top-up
    • Coinbase charge creation
  • Introduces team operations:

    • List members
    • Create invites
    • List invites
    • Revoke invites
    • Remove members
    • Change member roles
  • Wires new controllers and schemas into the core registry so methods are discoverable and callable via existing RPC/CLI surface

  • Expands unit and JSON-RPC E2E coverage to validate:

    • Request shapes
    • Method registration
    • Backend contract behavior (with mock upstream)

Problem

  • Billing and team workflows were not exposed through the openhuman core
  • Desktop app and adapters could not use shared JSON-RPC surface for these capabilities
  • Lack of schemas and registry wiring led to:
    • Ad hoc implementations
    • Harder validation and documentation
    • Inconsistent testing across adapters
  • Missing integration coverage increased risk of contract drift between:
    • Frontend
    • Core RPC layer
    • Backend APIs

Solution

  • Added dedicated modules:

    • src/openhuman/billing/
    • src/openhuman/team/
  • Implemented:

    • Thin backend-facing operations
    • Schema-driven controller registration
  • Followed existing architecture:

    • Business logic in core domain layer
    • Thin transport layer (no special-case HTTP logic)
  • Reused session JWT authentication for backend calls

  • Added validation and testing:

    • Schema validation in src/core/jsonrpc.rs
    • E2E RPC coverage in tests/json_rpc_e2e.rs

Submission Checklist

  • Unit tests

    • Added/updated cargo test coverage for:
      • Controller schemas
      • JSON-RPC validation paths
  • E2E / Integration tests

    • Added JSON-RPC integration coverage in:
      • tests/json_rpc_e2e.rs
    • Uses mock backend routes for billing and team flows
  • Doc comments

    • Added module-level Rust documentation for billing/team modules and operations
  • Inline comments

    • Added where flow/security context required clarification

Impact

  • Affects:

    • Rust core
    • JSON-RPC surface
    • Desktop/CLI consumers of these methods
  • Improves architecture consistency by:

    • Using shared controller registry instead of one-off adapters
  • Uses existing session JWT handling:

    • Reviewers should verify auth assumptions and backend contract compatibility
  • No migration required

  • New methods available:

    • openhuman.billing_*
    • openhuman.team_*

Related

Summary by CodeRabbit

  • New Features

    • Added billing management: view current plan, purchase plans, create portal sessions, top up credits, and create Coinbase charges.
    • Added team management: list members, create/revoke invites, remove members, and change member roles.
  • Tests

    • Added extensive unit and end-to-end tests and schema assertions covering billing and team RPC methods, parameter validation, and mock backend interactions.

- Introduced billing module with methods for fetching current plans, purchasing plans, creating portal sessions, and topping up credits.
- Added team management module with methods for listing team members, creating invites, listing invites, removing members, and changing member roles.
- Updated core registry to include new billing and team controllers and schemas, enhancing the overall functionality of the application.
- Implemented comprehensive tests for billing and team RPC methods to ensure reliability and correctness.

These additions improve the application's capabilities in managing billing and team functionalities, providing a more robust user experience.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 1, 2026

📝 Walkthrough

Walkthrough

Adds two new OpenHuman namespaces—billing and team—including HTTP adapter ops, controller schemas, registry wiring into core, expanded JSON-RPC tests, and e2e tests with a mock backend exercising JWT-authenticated calls.

Changes

Cohort / File(s) Summary
Core integration & tests
src/core/all.rs, src/core/jsonrpc.rs
Register openhuman::billing and openhuman::team controller schemas/registered controllers; extend JSON-RPC tests to assert presence and param validation for new billing/team methods.
Billing module
src/openhuman/billing/mod.rs, src/openhuman/billing/ops.rs, src/openhuman/billing/schemas.rs
New billing module: HTTP reqwest-based ops with JWT auth, request validation, envelope unwrapping; schema declarations, registered controllers, handlers, and unit tests.
Team module
src/openhuman/team/mod.rs, src/openhuman/team/ops.rs, src/openhuman/team/schemas.rs
New team module: HTTP ops for members/invites with JWT auth, input validation, envelope unwrapping; schema declarations, registered controllers, handlers, and unit tests.
Module declarations
src/openhuman/mod.rs
Exported new modules: pub mod billing; and pub mod team;.
E2E tests & mock backend
tests/json_rpc_e2e.rs
Extended Axum mock backend with billing and team routes (Bearer checks, payload validation) and added two e2e tests (billing_rpc_e2e, team_rpc_e2e) that run the real JSON-RPC server against the mock backend.

Sequence Diagram(s)

sequenceDiagram
    participant Client as JSON-RPC Client
    participant RPC as RPC Server
    participant Config as Config Manager
    participant Backend as Backend API

    Client->>RPC: openhuman.billing_get_current_plan()
    RPC->>Config: load_config_with_timeout()
    Config-->>RPC: Config + session JWT
    RPC->>Backend: GET /payments/stripe/currentPlan (Authorization: Bearer JWT)
    Backend-->>RPC: { "success": true, "data": {...} }
    RPC->>RPC: Unwrap envelope → data
    RPC-->>Client: RpcOutcome with plan details
Loading
sequenceDiagram
    participant Client as JSON-RPC Client
    participant RPC as RPC Server
    participant Config as Config Manager
    participant Backend as Backend API

    Client->>RPC: openhuman.team_list_members(teamId)
    RPC->>RPC: Validate teamId (trim, non-empty)
    RPC->>Config: load_config_with_timeout()
    Config-->>RPC: Config + session JWT
    RPC->>Backend: GET /teams/{teamId}/members (Authorization: Bearer JWT)
    Backend-->>RPC: { "success": true, "data": [...] }
    RPC->>RPC: Unwrap envelope → data
    RPC-->>Client: RpcOutcome with member array
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • Issue #99: Implements the requested “team & billing” feature by adding openhuman::billing and openhuman::team modules, ops, schemas, and registry wiring; tests and e2e coverage align with the issue objectives.

Poem

🐇 Coins and teams, I stitched the trail,

Tokens hum and routes set sail,
Schemas snug in camelCase clothes,
Backends answer, data glows,
A happy rabbit hops with glee.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.87% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main changes: adding billing and team management RPC functionality to the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/openhuman/mod.rs (1)

25-37: ⚠️ Potential issue | 🟡 Minor

Run cargo fmt on this module list.

CI is already red on rustfmt/prettier for this hunk, so the PR will keep failing until the declarations match formatter output.

As per coding guidelines, "Run cargo fmt and cargo check for Rust code changes before merging".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/mod.rs` around lines 25 - 37, The module declaration list (pub
mod billing; pub mod health; pub mod heartbeat; pub mod learning; pub mod
local_ai; pub mod memory; pub mod migration; pub mod providers; pub mod
screen_intelligence; pub mod security; pub mod service; pub mod skills; pub mod
team;) is not formatted to rustfmt standards; run cargo fmt (or rustfmt) on the
repository and commit the resulting changes so the declarations in mod.rs match
the formatter output (ensure the pub mod lines are reformatted exactly as
rustfmt produces).
🧹 Nitpick comments (1)
src/openhuman/billing/mod.rs (1)

6-12: Add a dedicated billing::rpc layer for the new controller flow.

Re-exporting ops::* from the domain root means the schema handlers end up calling implementation functions directly. That blurs the controller/implementation split and leaves no obvious place to host the debug/trace logging the repo expects on new billing paths.

As per coding guidelines, "Keep implementation in openhuman::/, controllers in openhuman::/rpc.rs, routes in core_server/", "Handlers in schemas.rs should delegate to existing domain rpc.rs functions during controller migration", and "Add substantial debug logging on new/changed flows using log/tracing at debug or trace level in Rust".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/billing/mod.rs` around lines 6 - 12, Currently ops::* is
re-exported from the domain root which lets schema handlers call implementation
functions directly; create a new rpc layer (e.g., add a billing::rpc module and
a rpc.rs exporting controller-facing functions) and change schemas.rs to
delegate to those rpc functions (call the new rpc::... handlers such as
billing::rpc::handle_* instead of ops::*) while adding debug/trace logs via
tracing::debug or tracing::trace at the entry/exit of each new/changed flow;
finally, stop re-exporting ops::* from mod.rs and instead re-export the rpc
surface (or explicitly export ops and rpc separately) so controllers live in rpc
and implementations stay in ops.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/openhuman/billing/ops.rs`:
- Around line 52-60: The file fails rustfmt; reformat the code (run cargo fmt)
so the request-building/await lines match project rustfmt rules—specifically
reformat the block that builds `req` using `client.request(method.clone(),
url.clone()).header(AUTHORIZATION, bearer_authorization_value(token));`, the
conditional `if let Some(b) = body { req = req.json(&b); }`, and the `let resp =
req.send().await.map_err(|e| format!("request failed: {e}"))?;` line; run `cargo
fmt` (and `cargo check` locally) and commit the formatted
`src/openhuman/billing/ops.rs` changes.
- Around line 195-202: Reject non-finite amount_usd values before proceeding: in
the top_up_credits function, add a check using amount_usd.is_finite() (or
equivalent is_nan/is_infinite checks) and return an Err like "amountUsd must be
a finite number" if it fails, keeping the existing <= 0.0 validation afterward
so NaN/Infinity are blocked from being serialized into the billing payload.
- Around line 60-66: The code currently uses
resp.text().await.unwrap_or_default(), which hides I/O errors reading the
response body; change this to propagate the read error instead of turning
failures into an empty string. Replace that call with something like
resp.text().await.map_err(|e| format!("failed to read response body for {} {}:
{e}", method, url))? so the function returns an Err on text-read failures
(include method/url and status context if helpful) before attempting JSON
parsing into raw; keep the existing serde_json::from_str handling but do not
swallow resp.text() errors.

In `@src/openhuman/billing/schemas.rs`:
- Around line 79-128: The published ControllerSchema outputs for
billing_purchase_plan (purchase_plan), billing_create_portal_session
(create_portal_session), billing_top_up (top_up), and
billing_create_coinbase_charge (create_coinbase_charge) use wrapper field names
(“session”, “topUp”, “charge”) but the handlers return the backend JSON directly
and E2E tests read fields like checkoutUrl, portalUrl, amountUsd, and hostedUrl;
update each ControllerSchema outputs to reflect the actual returned JSON shape
(e.g., replace json_output("session", ...) with outputs that declare the real
top-level keys returned by the backend such as "checkoutUrl" for purchase_plan,
"portalUrl" for create_portal_session, "amountUsd" for top_up, and "hostedUrl"
for create_coinbase_charge—or alternatively document a single raw JSON root) so
the /schema contract matches the real responses.

In `@src/openhuman/team/ops.rs`:
- Around line 136-139: The code builds request paths by directly formatting
trimmed IDs into strings (e.g., in list_members and the related functions that
build paths with team_id, user_id, invite_id), which allows reserved characters
to change the endpoint; instead, parse the base URL (or construct a base like
"/teams") with the url::Url type and append user-provided segments via
Url::path_segments_mut().unwrap().push(&team_id); push(&user_id) etc., so the
url crate percent-encodes reserved characters; update every location that does
format!("/teams/{}/members", team_id) (and the other occurrences noted) to use
Url path_segments_mut push() and then obtain the encoded path string to pass
into get_authed_value.
- Around line 77-83: The code currently swallows response-body I/O errors by
using resp.text().await.unwrap_or_default(), so change the read to propagate the
error instead of defaulting: replace the unwrap_or_default on resp.text().await
with proper error propagation (e.g., use .await.map_err(|e| format!("failed
reading response body: {e}"))? or the equivalent) so body read/decoding failures
bubble up; keep the existing serde_json::from_str(&text).unwrap_or_else(|_|
Value::String(text.clone())) behavior for JSON decoding fallback (the variables
to edit are resp.text().await, text and the subsequent serde_json::from_str
producing raw).

In `@src/openhuman/team/schemas.rs`:
- Around line 90-163: Update the ControllerSchema outputs to match the actual
handler return shapes: for "team_list_members" (function list_members) and
"team_list_invites" (function list_invites) replace the current object-wrapped
outputs with a JSON array output schema (i.e., reflect that handlers return a
raw array of members/ invites), and for "team_create_invite" (function
create_invite) change the output to the direct invite object (not a wrapper like
{invite: ...}) so the RPC /schema matches the handlers' returned types.

In `@tests/json_rpc_e2e.rs`:
- Around line 1113-1118: The helper inner() currently assumes every RPC response
is an object with a "result" field and panics when given the bare value returned
by into_cli_compatible_json(); update inner(outer: &Value, ctx: &str) to accept
both shapes by returning outer.get("result").cloned().unwrap_or_else(||
outer.clone()) (or equivalent) so it returns the "result" when present and
otherwise returns the outer value itself; apply the same change to the other
inner helper referenced (the one around lines 1258–1263).
- Around line 87-175: The mock handlers (stripe_current_plan,
stripe_purchase_plan, stripe_portal, credits_top_up, coinbase_charge,
team_members, team_invites_get, team_invites_post, team_member_delete,
team_member_role_put, team_invite_delete) should validate the Authorization
bearer token and required request payload fields and return an error JSON/status
if missing or invalid; update each async handler to extract the Authorization
header (or the Json body where applicable), check against the expected mock
token/session and required fields (e.g., sessionId/checkoutUrl for
stripe_purchase_plan, body contents for credits_top_up/coinbase_charge, invite
payload for team_invites_post), and respond with a 401/400-style mock failure
JSON when validation fails, otherwise return the existing success response.
Ensure you reference and update the handler functions named above so tests will
fail when auth or payloads are not forwarded correctly.

---

Outside diff comments:
In `@src/openhuman/mod.rs`:
- Around line 25-37: The module declaration list (pub mod billing; pub mod
health; pub mod heartbeat; pub mod learning; pub mod local_ai; pub mod memory;
pub mod migration; pub mod providers; pub mod screen_intelligence; pub mod
security; pub mod service; pub mod skills; pub mod team;) is not formatted to
rustfmt standards; run cargo fmt (or rustfmt) on the repository and commit the
resulting changes so the declarations in mod.rs match the formatter output
(ensure the pub mod lines are reformatted exactly as rustfmt produces).

---

Nitpick comments:
In `@src/openhuman/billing/mod.rs`:
- Around line 6-12: Currently ops::* is re-exported from the domain root which
lets schema handlers call implementation functions directly; create a new rpc
layer (e.g., add a billing::rpc module and a rpc.rs exporting controller-facing
functions) and change schemas.rs to delegate to those rpc functions (call the
new rpc::... handlers such as billing::rpc::handle_* instead of ops::*) while
adding debug/trace logs via tracing::debug or tracing::trace at the entry/exit
of each new/changed flow; finally, stop re-exporting ops::* from mod.rs and
instead re-export the rpc surface (or explicitly export ops and rpc separately)
so controllers live in rpc and implementations stay in ops.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3a03a326-d0a7-4a75-9692-4d10b4bf2161

📥 Commits

Reviewing files that changed from the base of the PR and between 7fd4568 and e0049f5.

📒 Files selected for processing (10)
  • src/core/all.rs
  • src/core/jsonrpc.rs
  • src/openhuman/billing/mod.rs
  • src/openhuman/billing/ops.rs
  • src/openhuman/billing/schemas.rs
  • src/openhuman/mod.rs
  • src/openhuman/team/mod.rs
  • src/openhuman/team/ops.rs
  • src/openhuman/team/schemas.rs
  • tests/json_rpc_e2e.rs

Comment thread src/openhuman/billing/ops.rs Outdated
Comment thread src/openhuman/billing/ops.rs Outdated
Comment thread src/openhuman/billing/ops.rs
Comment thread src/openhuman/billing/schemas.rs Outdated
Comment thread src/openhuman/team/ops.rs Outdated
Comment thread src/openhuman/team/ops.rs
Comment thread src/openhuman/team/schemas.rs
Comment thread tests/json_rpc_e2e.rs Outdated
Comment thread tests/json_rpc_e2e.rs Outdated
- Improved error handling in  to provide clearer error messages when reading response bodies.
- Updated validation in  to ensure  is a finite number greater than zero.
- Refined output schemas for billing and team functions to include more descriptive fields, enhancing API clarity.
- Introduced a new  function to standardize URL path construction, improving code maintainability.
- Added tests to verify the correctness of new output structures and API path building.

These changes enhance the robustness and usability of the billing and team management functionalities.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/openhuman/billing/schemas.rs (1)

67-177: ⚠️ Potential issue | 🟠 Major

Publish the billing response shape under result, not at the top level.

All of the billing handlers below call to_json(...) on RpcOutcome::single_log(...), so callers receive { "result": <payload>, "logs": [...] }. Advertising checkoutUrl, portalUrl, amountUsd, and similar fields as top-level outputs makes /schema point consumers at the wrong JSON path. At minimum, these payload fields need to be nested under a top-level result field.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/billing/schemas.rs` around lines 67 - 177, The schema currently
lists payload fields (e.g., checkoutUrl, portalUrl, amountUsd,
gatewayTransactionId, hostedUrl, status, expiresAt) as top-level outputs for
ControllerSchema entries like billing_purchase_plan,
billing_create_portal_session, billing_top_up, billing_create_coinbase_charge
and billing_get_current_plan; update each ControllerSchema so that its outputs
instead expose a single top-level "result" output (e.g., output_field("result",
TypeSchema::/*appropriate object/JSON type*/, "Result payload containing the
fields previously listed at top level")) and move the previous per-field output
descriptors into the nested shape described by that result field so /schema
correctly points consumers to { "result": <payload>, "logs": [...] } as returned
by RpcOutcome::single_log/to_json.
🧹 Nitpick comments (2)
src/openhuman/team/ops.rs (1)

25-159: Extract the shared authed-request helper before billing and team drift further.

build_client, resolve_base, require_token, authed_request, unwrap_api_envelope, and get_authed_value are nearly identical to src/openhuman/billing/ops.rs. Future fixes to auth, envelope parsing, or timeout behavior will be easy to land in one module and miss in the other.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/team/ops.rs` around lines 25 - 159, This code duplicates
HTTP/auth helpers already present in billing; extract build_client,
resolve_base, require_token, authed_request, unwrap_api_envelope, and
get_authed_value into a single shared module (e.g., a new openhuman::ops or
client helper module), move the common implementations there, and have both team
and billing modules call the shared functions instead of their local copies;
ensure public signatures match (keeping Method, Client, Url, and
serde_json::Value usage) and update imports (use statements) in
src/openhuman/team/ops.rs and the billing ops file to reference the shared
functions so future changes to auth/envelope/timeout are applied in one place.
src/openhuman/team/schemas.rs (1)

183-255: Consider moving the handler orchestration behind openhuman::team::rpc.

Right now schemas.rs is doing config load, param decode, and dispatch itself. Shifting that into a dedicated team::rpc layer would keep this module metadata-focused and align the new domain with the repo's controller boundary.

As per coding guidelines, "Handlers in schemas.rs should delegate to existing domain rpc.rs functions during controller migration".

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/openhuman/billing/ops.rs`:
- Around line 194-209: In top_up_credits, validate the optional gateway string
before building TopUpBody: trim and normalize (e.g., to_lowercase) the gateway
received in top_up_credits, check it against the allowed set
{"stripe","coinbase"}, and if it is non-empty and not one of those, return Err
with a clear validation message listing supported values; only then populate
TopUpBody.gateway (using default_gateway when gateway is empty/None). This
prevents forwarding typos to the backend and provides immediate local
validation.

In `@src/openhuman/team/ops.rs`:
- Line 106: The current debug! call logs the full URL (including team_id,
user_id, invite_id); change it to log a redacted route or template instead.
Implement a helper (e.g., redact_url or redact_route_template) that replaces ID
segments (numeric or UUID-like) in the passed url with placeholders like
"{team_id}", "{user_id}", "{invite_id}" or a generic "{id}", and call that
helper in place of url in the debug! call (the debug! using LOG_PREFIX, method,
url, status). Ensure the helper is used wherever request URLs are logged in
ops.rs so raw IDs are never emitted.

---

Duplicate comments:
In `@src/openhuman/billing/schemas.rs`:
- Around line 67-177: The schema currently lists payload fields (e.g.,
checkoutUrl, portalUrl, amountUsd, gatewayTransactionId, hostedUrl, status,
expiresAt) as top-level outputs for ControllerSchema entries like
billing_purchase_plan, billing_create_portal_session, billing_top_up,
billing_create_coinbase_charge and billing_get_current_plan; update each
ControllerSchema so that its outputs instead expose a single top-level "result"
output (e.g., output_field("result", TypeSchema::/*appropriate object/JSON
type*/, "Result payload containing the fields previously listed at top level"))
and move the previous per-field output descriptors into the nested shape
described by that result field so /schema correctly points consumers to {
"result": <payload>, "logs": [...] } as returned by
RpcOutcome::single_log/to_json.

---

Nitpick comments:
In `@src/openhuman/team/ops.rs`:
- Around line 25-159: This code duplicates HTTP/auth helpers already present in
billing; extract build_client, resolve_base, require_token, authed_request,
unwrap_api_envelope, and get_authed_value into a single shared module (e.g., a
new openhuman::ops or client helper module), move the common implementations
there, and have both team and billing modules call the shared functions instead
of their local copies; ensure public signatures match (keeping Method, Client,
Url, and serde_json::Value usage) and update imports (use statements) in
src/openhuman/team/ops.rs and the billing ops file to reference the shared
functions so future changes to auth/envelope/timeout are applied in one place.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c2a44e7e-6289-43f1-b5cc-f7952cde0076

📥 Commits

Reviewing files that changed from the base of the PR and between e0049f5 and ba108e4.

📒 Files selected for processing (7)
  • src/core/jsonrpc.rs
  • src/openhuman/billing/ops.rs
  • src/openhuman/billing/schemas.rs
  • src/openhuman/mod.rs
  • src/openhuman/team/ops.rs
  • src/openhuman/team/schemas.rs
  • tests/json_rpc_e2e.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/openhuman/mod.rs
  • tests/json_rpc_e2e.rs

Comment thread src/openhuman/billing/ops.rs
Comment thread src/openhuman/team/ops.rs Outdated
…nctionality

- Introduced  function to standardize payment gateway inputs, ensuring only valid options (stripe, coinbase) are accepted, with defaults and error handling.
- Enhanced  function to utilize the new gateway normalization logic, improving input validation.
- Added  function to obscure sensitive identifiers in API route paths, enhancing security in logging.
- Implemented unit tests for both  and  to ensure correctness and reliability of the new features.

These changes improve the robustness of billing operations and enhance security in route handling.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/openhuman/billing/ops.rs`:
- Around line 35-38: The resolve_base function currently accepts any URL scheme
and can drop path prefixes when used with Url::join; update resolve_base (which
calls effective_api_url and Url::parse) to validate the scheme (only allow
"https" except allow "http" for loopback/localhost addresses) and return a clear
Err on invalid schemes, and ensure the returned base URL preserves a trailing
slash or otherwise normalizes to a directory-style base so subsequent Url::join
calls (used where the Authorization header is set) keep existing path segments
(e.g., append '/' if missing or use Url::parse with base + "/") to prevent
dropping a prefix like "/api". Ensure error messages include the invalid URL and
reason.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 811d63ce-263f-49bf-826f-0b1dc76caa99

📥 Commits

Reviewing files that changed from the base of the PR and between ba108e4 and 7bba5e1.

📒 Files selected for processing (2)
  • src/openhuman/billing/ops.rs
  • src/openhuman/team/ops.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/openhuman/team/ops.rs

Comment on lines +35 to +38
fn resolve_base(config: &Config) -> Result<Url, String> {
let base = effective_api_url(&config.api_url);
Url::parse(base.trim()).map_err(|e| format!("invalid api_url '{}': {e}", base))
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Expectation:
# 1) src/api/config.rs strips trailing slashes from configured API bases.
# 2) Relative URL resolution then drops a trailing path segment like `/api`
#    unless the base ends with `/`.

rg -n -C2 'fn normalize_api_base_url|trim_end_matches' src/api/config.rs

python - <<'PY'
from urllib.parse import urljoin
base = "https://proxy.example.com/api"
path = "payments/stripe/currentPlan"
print("without trailing slash:", urljoin(base, path))
print("with trailing slash   :", urljoin(base + "/", path))
PY

Repository: tinyhumansai/openhuman

Length of output: 413


🏁 Script executed:

# Check if the file exists and examine authed_request function
fd -t f "ops.rs" src/openhuman/billing/

Repository: tinyhumansai/openhuman

Length of output: 94


🏁 Script executed:

# Read the authed_request function to see how it uses resolve_base
rg -n -A15 "fn authed_request" src/openhuman/billing/ops.rs

Repository: tinyhumansai/openhuman

Length of output: 542


🏁 Script executed:

# Verify resolve_base is defined and how it's called
rg -n -B2 -A8 "fn resolve_base" src/openhuman/billing/ops.rs

Repository: tinyhumansai/openhuman

Length of output: 376


🏁 Script executed:

# Check the effective_api_url function definition
rg -n -B3 -A5 "fn effective_api_url" src/api/config.rs

Repository: tinyhumansai/openhuman

Length of output: 556


🏁 Script executed:

# Check if there are any scheme validations elsewhere in the billing module
rg -n "http|https|scheme" src/openhuman/billing/ops.rs

Repository: tinyhumansai/openhuman

Length of output: 90


🏁 Script executed:

# Check if resolve_base result is validated or constrained elsewhere before use
rg -n "resolve_base" src/openhuman/billing/ops.rs

Repository: tinyhumansai/openhuman

Length of output: 168


🏁 Script executed:

# Check the broader context: how is Config created/validated in this flow?
rg -n "Config" src/openhuman/billing/ops.rs | head -20

Repository: tinyhumansai/openhuman

Length of output: 592


🏁 Script executed:

# Check context around line 135 where resolve_base is called
sed -n '125,145p' src/openhuman/billing/ops.rs

Repository: tinyhumansai/openhuman

Length of output: 674


🏁 Script executed:

# Also check if there's any environment-based restriction on api_url scheme
rg -n "BACKEND_URL|VITE_BACKEND_URL|api_url" src/api/config.rs | head -20

Repository: tinyhumansai/openhuman

Length of output: 610


🏁 Script executed:

# Verify the DEFAULT_API_BASE_URL to understand default scheme
rg -n "DEFAULT_API_BASE_URL" src/api/config.rs

Repository: tinyhumansai/openhuman

Length of output: 274


🏁 Script executed:

# Check if there's any validation of config.api_url elsewhere during initialization
rg -n "api_url" src/openhuman/config.rs | head -20

Repository: tinyhumansai/openhuman

Length of output: 129


🏁 Script executed:

# Find where Config struct is defined
rg -n "struct Config" src/openhuman/ -t rs

Repository: tinyhumansai/openhuman

Length of output: 92


🏁 Script executed:

# Check the import in ops.rs to understand which Config module is used
head -30 src/openhuman/billing/ops.rs | rg -n "use.*Config"

Repository: tinyhumansai/openhuman

Length of output: 106


🏁 Script executed:

# Find Config struct definition
rg -n "struct Config" src/openhuman/

Repository: tinyhumansai/openhuman

Length of output: 198


🏁 Script executed:

# List files in src/openhuman to find config.rs
ls -la src/openhuman/*.rs

Repository: tinyhumansai/openhuman

Length of output: 282


🏁 Script executed:

# Check the Config struct definition and api_url field
sed -n '1,50p' src/openhuman/config/schema/types.rs

Repository: tinyhumansai/openhuman

Length of output: 1417


🏁 Script executed:

# Check if api_url has any constraints or validation
rg -n "api_url" src/openhuman/config/schema/types.rs -A3 -B1

Repository: tinyhumansai/openhuman

Length of output: 454


🏁 Script executed:

# Check if there's any validation or interception of http:// urls in the build process
rg -n "http://" src/ --max-count=5

Repository: tinyhumansai/openhuman

Length of output: 4817


🏁 Script executed:

# Verify the exact line numbers for authed_request and where it's called from get_authed_value
sed -n '125,155p' src/openhuman/billing/ops.rs

Repository: tinyhumansai/openhuman

Length of output: 938


Harden resolve_base() to validate scheme and preserve path segments.

Line 37 accepts any URL scheme without restriction. When a non-loopback http:// override is configured, the session JWT is transmitted in the Authorization header (line 54) over cleartext. Additionally, line 49 uses Url::join() against a base that effective_api_url() has already stripped of its trailing slash, causing path segments like /api to be dropped—https://proxy.example.com/api will resolve to /payments/... instead of /api/payments/..., breaking reverse-proxy deployments with a path prefix.

Suggested fix
 fn resolve_base(config: &Config) -> Result<Url, String> {
     let base = effective_api_url(&config.api_url);
-    Url::parse(base.trim()).map_err(|e| format!("invalid api_url '{}': {e}", base))
+    let mut url =
+        Url::parse(base.trim()).map_err(|e| format!("invalid api_url '{}': {e}", base))?;
+
+    let is_loopback = matches!(url.host_str(), Some("localhost" | "127.0.0.1" | "::1"));
+    match (url.scheme(), is_loopback) {
+        ("https", _) | ("http", true) => {}
+        ("http", false) => {
+            return Err("api_url must use https (http is only allowed for localhost)".to_string())
+        }
+        (scheme, _) => return Err(format!("unsupported api_url scheme '{scheme}'")),
+    }
+
+    if !url.path().ends_with('/') {
+        let new_path = format!("{}/", url.path());
+        url.set_path(&new_path);
+    }
+
+    Ok(url)
 }

Also applies to: 48-54

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/billing/ops.rs` around lines 35 - 38, The resolve_base function
currently accepts any URL scheme and can drop path prefixes when used with
Url::join; update resolve_base (which calls effective_api_url and Url::parse) to
validate the scheme (only allow "https" except allow "http" for
loopback/localhost addresses) and return a clear Err on invalid schemes, and
ensure the returned base URL preserves a trailing slash or otherwise normalizes
to a directory-style base so subsequent Url::join calls (used where the
Authorization header is set) keep existing path segments (e.g., append '/' if
missing or use Url::parse with base + "/") to prevent dropping a prefix like
"/api". Ensure error messages include the invalid URL and reason.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Team & billing in openhuman binary (RPC/CLI: plans, top-ups, payment links, members)

2 participants