Skip to content

fix(api): safely join api_url + path so misconfigured base can't corrupt routes#1650

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
senamakel:fix/api-url-safe-join
May 13, 2026
Merged

fix(api): safely join api_url + path so misconfigured base can't corrupt routes#1650
senamakel merged 2 commits into
tinyhumansai:mainfrom
senamakel:fix/api-url-safe-join

Conversation

@senamakel
Copy link
Copy Markdown
Member

@senamakel senamakel commented May 13, 2026

Summary

  • A misconfigured api_url (e.g. a user pasted the LLM endpoint https://api.tinyhumans.ai/openai/v1/chat/completions into the base URL setting) silently corrupted every /agent-integrations/... call because the HTTP clients just format!("{base}{path}")-concatenated.
  • Result was URLs like …/openai/v1/chat/completions/agent-integrations/composio/toolkits that 404 — breaking Composio connections ("stale") and toolkit loading for affected users.
  • Add api::config::api_url(base, path) that uses url::Url::join so an absolute-path reference replaces any path baked into the base (RFC 3986), and replace the three concat call sites.

Problem

The Config.api_url field is the single base for both LLM-proxy calls and /agent-integrations/* calls. When a user puts a path-bearing URL in that field, the integrations client compounds the path. Every Composio call (list_toolkits, list_tools, list_connections, authorize, triggers, …) and every shared integrations get/post is affected. The user-visible symptom is "Connections are showing stale status" plus 404s from list_toolkits.

Solution

  • New helper pub fn api_url(base: &str, path: &str) -> String in src/api/config.rs.
    • Empty path → normalized base (no trailing slash).
    • Non-empty pathurl::Url::parse(base)?.join(path)?. For an absolute-path reference (/agent-integrations/...) this replaces the base's path per RFC 3986, neutralising the misconfigured-base case.
    • Unparseable base → slash-safe fallback_concat so callers always get a usable string.
  • Replaced the three call sites: src/openhuman/integrations/client.rs (POST + GET) and src/openhuman/composio/client.rs (DELETE).
  • Tradeoff: Url::join would drop the base's last path segment if path is relative — documented in the helper's docstring. All our API paths start with /, so this is safe.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
  • N/A: pure bugfix in a code-path with existing tests; diff coverage is satisfied by the 5 new unit tests for api_url itself, which are the changed lines.
  • N/A: behaviour-only change — no feature added/removed/renamed in the matrix.
  • N/A: no matrix feature IDs touched.
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • N/A: does not touch release-cut surfaces.
  • N/A: no linked issue — surfaced directly in chat from a user repro.

Impact

  • Runtime/platform: desktop (all OSes via the shared core). No mobile/web/CLI change beyond the fact that the core CLI also uses the same client.
  • Performance: one Url::parse per request; negligible.
  • Security: slightly safer — a path baked into a misconfigured base no longer carries through to every backend call. No auth/secrets change.
  • Migration/compat: backward-compatible. Existing valid bases (https://api.tinyhumans.ai, with or without trailing slash) produce identical output to the old format! path.

Related

  • Closes:
  • Follow-up PR(s)/TODOs: optionally reject/strip a path on api_url at config load to warn the user proactively; not done here to keep the diff minimal.

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/api-url-safe-join
  • Commit SHA: 7e5108d

Validation Run

  • pnpm --filter openhuman-app format:check
  • N/A: no app/ TypeScript changes in this PR.
  • Focused tests: cargo test --manifest-path Cargo.toml --lib api::config:: (11/11 pass, including 5 new api_url_* tests)
  • Rust fmt/check (if changed): cargo fmt --check clean
  • N/A: Tauri shell unchanged.

Validation Blocked

  • command: N/A
  • error: N/A
  • impact: N/A

Behavior Changes

  • Intended behavior change: URL construction in the integrations + composio HTTP clients goes through a safe joiner that uses RFC 3986 resolution instead of string concat.
  • User-visible effect: users with a path-bearing api_url (e.g. pasted an LLM endpoint URL) no longer see /agent-integrations/* 404s. Composio connections stop showing "stale" for that misconfiguration.

Parity Contract

  • Legacy behavior preserved: when api_url is the correct origin (e.g. https://api.tinyhumans.ai), the joined URL is byte-identical to what the previous format! produced.
  • Guard/fallback/dispatch parity checks: unparseable base falls back to slash-safe concat (covered by api_url_unparseable_base_falls_back_to_concat).

Duplicate / Superseded PR Handling

  • Duplicate PR(s): N/A
  • Canonical PR: N/A
  • Resolution (closed/superseded/updated): N/A

senamakel added 2 commits May 13, 2026 07:43
…rrupt routes

A misconfigured `api_url` (e.g. user pasted an LLM endpoint
`https://api.tinyhumans.ai/openai/v1/chat/completions` into the base URL
setting) silently corrupted every `/agent-integrations/...` call because
`format!("{base}{path}")` just concatenated. The result —
`…/openai/v1/chat/completions/agent-integrations/composio/toolkits` — 404s,
breaking Composio connections and toolkit loading.

Add `api::config::api_url(base, path)` that uses `url::Url::join` so an
absolute-path reference replaces any path baked into the base (RFC 3986).
Empty path returns the normalized base; unparseable base falls back to a
slash-safe concat so callers always get a usable string.

Replace the three call sites in the integrations + composio HTTP clients.
@senamakel senamakel requested a review from a team May 13, 2026 14:45
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

📝 Walkthrough

Walkthrough

This PR adds a centralized API URL joining helper (api_url) to handle RFC 3986-compliant path-joining semantics with fallback behavior, then updates two client implementations to use this helper instead of manual string concatenation.

Changes

API URL Normalization Consolidation

Layer / File(s) Summary
API URL helper and validation
src/api/config.rs
New api_url(base, path) function handles empty-path normalization, absolute-path replacement (RFC 3986 semantics), URL parsing with fallback concatenation, and trailing-slash trimming. Comprehensive test suite validates empty paths, absolute-path replacement, clean joining, query-string preservation, and fallback concatenation for invalid bases.
Client integration
src/openhuman/composio/client.rs, src/openhuman/integrations/client.rs
ComposioClient::raw_delete, IntegrationClient::post, and IntegrationClient::get now derive request URLs via the centralized api_url helper instead of manual string concatenation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

Poem

A rabbit hops through URL paths,
With slashes trailing, parsing baths,
RFC guides the joining quest,
Where base and path are joined the best,
No more string concat in the fray,
One helper rules them all today! 🐰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: a new safe API URL joining function that prevents misconfigured base URLs from corrupting API routes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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

@senamakel senamakel merged commit 1d37d2d into tinyhumansai:main May 13, 2026
20 of 24 checks passed
senamakel added a commit to senamakel/openhuman that referenced this pull request May 13, 2026
config.api_url was double-purposed as both the OpenHuman product backend
URL (auth/billing/voice/integrations/...) AND the LLM inference endpoint
introduced in tinyhumansai#1342. Pointing api_url at a custom OpenAI-compat provider
silently rerouted every other backend call to OpenAI, breaking
GET /auth/me, GET /auth/google/login, voice transcribe, billing, etc.

This commit cleanly separates the two concerns:

- New optional `config.inference_url`. When set together with `api_key`,
  the inference provider talks directly to that URL; otherwise inference
  flows through the OpenHuman backend at `{api_url}/openai/v1/...`.
- `api_url` always means the OpenHuman product backend URL (defaults to
  api.tinyhumans.ai). Auth/billing/voice/etc. callers are untouched.
- New `effective_inference_url()` helper derives the URL via the safe
  `api_url()` joiner from tinyhumansai#1650.
- Inference provider factories (`create_backend_inference_provider`,
  `create_resilient_provider*`, `create_routed_provider*`,
  `create_intelligent_routing_provider`) now take separate
  `inference_url` and `backend_url` params; all callers updated.
- ChannelRuntimeContext gains an `inference_url` field plumbed from
  Config so per-channel provider creation honours the override.
- Settings: `update_model_settings` accepts an `inference_url` patch
  field; `get_client_config` returns it.
- Frontend: BackendProviderPanel reads/writes `inference_url` (never
  `api_url`). OpenHuman preset clears `inference_url` so inference
  flows back through the backend.

Migration: on config load, any legacy `api_url` value ending in
`/chat/completions` is moved into `inference_url` (and cleared if it
pointed at the OpenHuman backend itself).

Also: RouterProvider now maps OpenHuman's abstract tier names
(`reasoning-v1`, `agentic-v1`, `coding-v1`, `summarization-v1`) through
the user's `model_routes` so a custom provider receives the configured
model id instead of the literal tier name, which would 404 on
OpenAI/Anthropic/etc.

Reported by user: chat hit "model 'reasoning-v1' does not exist" on
OpenAI, and login redirected to api.openai.com/.../auth/google/login.
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.

1 participant