diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a01309cc..7906cb02b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -741,6 +741,27 @@ jobs: cargo test --locked --features acp --lib server::acp cargo test --locked --features acp --lib harness::acp_run_turn + # Issue #788: Chargebee billing — the REST layer (`chargebee::`) and the + # toolbelt bridge (`harness::chargebee`) in one filter, which selects both. + # It runs HERE rather than on the fast default job because the bridge lives + # under `src/harness/` and so compiles only with `openhuman`; splitting it + # across two lanes would need two rows in feature-lanes.txt for one + # feature, which that table rejects. All of it is offline: the wire tests + # drive a stub on an ephemeral port and the rest never build a request. + - name: Test the Chargebee billing integration + run: scripts/ci/run-scoped-suite.sh "chargebee" openhuman,tinycortex,chargebee chargebee + + # Issue #789: PayPal. Here rather than on the fast default job for the same + # reason as chargebee above — the toolbelt bridge is under `src/harness/` + # and so needs `openhuman`. It USED to ride the default job on `--features + # paypal` alone, which reached `company::paypal` and `paypal::` and could + # not compile `harness::paypal` at all: those tests existed and no lane ran + # them, which is exactly the #770 pathology this table exists to stop. One + # lane with the gated feature set selects all three. Offline: the token + # cache and error tests drive a stub on an ephemeral port. + - name: Test the PayPal integration + run: scripts/ci/run-scoped-suite.sh "paypal" openhuman,tinycortex,paypal paypal + - name: Assert every integration target actually runs run: scripts/ci/assert-integration-targets-run.sh openhuman,tinycortex diff --git a/.github/workflows/deploy-staging.yml b/.github/workflows/deploy-staging.yml index c8a7b3a10..3fc0fbe2b 100644 --- a/.github/workflows/deploy-staging.yml +++ b/.github/workflows/deploy-staging.yml @@ -66,7 +66,7 @@ env: # medulla (hosted brain transport), tinycortex (local memory backend), and # sidecar. `sqlite` is intentionally omitted — its bundled rusqlite needs a C # compiler the slim build image lacks, and it is redundant on a mongodb tenant. - TENANT_FEATURES: mongodb,openhuman,openhuman-rpc,tinyplace,github,smtp,dns,tinyhumans,webhooks,platform-jwt,export,mcp,media,composio,imap,telegram,medulla,tinycortex,sidecar + TENANT_FEATURES: mongodb,openhuman,openhuman-rpc,tinyplace,github,smtp,dns,tinyhumans,webhooks,platform-jwt,export,mcp,media,composio,imap,telegram,medulla,tinycortex,sidecar,chargebee,paypal jobs: deploy: diff --git a/Cargo.toml b/Cargo.toml index 505ff93e6..5b34ff84c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -405,6 +405,16 @@ telegram = ["dep:reqwest"] # read-only connections catalog compiles without this; only the token-exchanging # write routes are gated here (they need the shared `reqwest` client). oauth = ["dep:reqwest"] +# Issue #788: Chargebee billing integrated as backend service code, surfaced to +# agents as callable tools. NOT a separate MCP server and NOT a second binary — +# the integration lives in the tenant workload so the credential can be +# per-company, held in that company's own SecretStore. The only gated dependency +# is the shared `reqwest` client the REST calls use; `src/chargebee/` compiles +# only under this feature, so the default build is unaffected. +chargebee = ["dep:reqwest"] +# Issue #789: PayPal wallet + transaction reads, same shape as `chargebee` +# above — backend service code, per-company credentials, only `reqwest` gated. +paypal = ["dep:reqwest"] [patch.crates-io] # opencompany's own vendored tinyagents (2.1.0). Satisfies openhuman's `^2.1`. diff --git a/companies/openhuman_demo/agents/ceo.toml b/companies/openhuman_demo/agents/ceo.toml index b8e568cdc..8fc70d9e2 100644 --- a/companies/openhuman_demo/agents/ceo.toml +++ b/companies/openhuman_demo/agents/ceo.toml @@ -6,4 +6,4 @@ description = "Sets direction, answers about the company, and delegates the work tier = "orchestrator" # `workspace.read` is read-only by design: writes need a bare `workspace` or an # explicit `workspace.write`, so this cannot silently overwrite operator notes. -tools = ["mcp:*", "workspace.read"] +tools = ["mcp:*", "workspace.read", "chargebee", "paypal"] diff --git a/companies/openhuman_demo/company.toml b/companies/openhuman_demo/company.toml index 3dcaf2b3a..a6cde17ab 100644 --- a/companies/openhuman_demo/company.toml +++ b/companies/openhuman_demo/company.toml @@ -31,7 +31,7 @@ mode = "full" # Both forms are needed: this list is matched with exact/glob semantics, so bare # `workspace` covers an agent asking for `workspace` but NOT one asking for # `workspace.read`, and `workspace.*` covers the sub-grants but not the bare one. -allow = ["mcp:*", "workspace", "workspace.*"] +allow = ["mcp:*", "workspace", "workspace.*", "chargebee", "paypal"] # Desks (group chats) the orchestrator can hand a turn to. Each desk's first # member is its lead; `delegate_to_desk` runs that member's turn. diff --git a/examples/live_company_turn.rs b/examples/live_company_turn.rs index b57dac1c0..c9875e30e 100644 --- a/examples/live_company_turn.rs +++ b/examples/live_company_turn.rs @@ -137,6 +137,10 @@ async fn main() -> anyhow::Result<()> { plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: opencompany::company::steer::InflightRegistry::default(), run_supervisor: opencompany::runtime::RunSupervisor::default(), delivery: None, diff --git a/frontend/src/api/billing.ts b/frontend/src/api/billing.ts new file mode 100644 index 000000000..8ee579677 --- /dev/null +++ b/frontend/src/api/billing.ts @@ -0,0 +1,121 @@ +// The Chargebee billing configuration API (issue #788, UI tracked in #527). +// +// Credentials are write-only: the API key and the webhook credential are sent +// on save and stored in the host's secret store; neither is ever returned. The +// read shape carries booleans and the (non-secret) site identifier only, so +// there is no field on this type that could leak a key into a rendered page. +// +// Standalone functions over the shared client, mirroring `api/mcp.ts` and +// `api/skills.ts`, so `OpenCompanyClient` needs no new methods. + +import type { OpenCompanyClient } from "./client"; + +/** + * The non-secret view of a company's Chargebee configuration. + * + * Four separate flags rather than one `connected`, because they fail + * differently and a single boolean sends an operator to the wrong place for + * three of them — see `BillingView` for how each is worded. + */ +export interface BillingStatus { + /** Whether an API key is stored. Never the key. */ + apiKeyConfigured: boolean; + /** The Chargebee site slug, e.g. `acme-test`. Not secret. */ + site: string | null; + /** Whether a webhook credential is stored. */ + webhookConfigured: boolean; + /** The URL to paste into Chargebee, or null on a host with no public URL. */ + webhookUrl: string | null; + /** Whether the company's manifest explicitly grants `chargebee`. */ + granted: boolean; + /** Whether the `chargebee` feature is compiled into the running host. */ + inBuild: boolean; +} + +/** The write-only save body. Omitted fields keep their stored value. */ +export interface BillingConfig { + /** Write-only. Omit to leave the stored key unchanged. */ + apiKey?: string; + /** The site identifier; accepts a bare slug, a host, or a full URL. */ + site?: string; + /** Write-only `username:password` pair. Omit to leave it unchanged. */ + webhookSecret?: string; +} + +/** Reads the company's Chargebee configuration status. */ +export async function getBilling( + client: OpenCompanyClient, + company: string | null, +): Promise { + return client.get(`${client.scopeFor(company)}/billing/chargebee`); +} + +/** + * Saves whatever is supplied, and returns the resulting status. + * + * A patch, not a replace: the host applies only the fields present and + * non-empty, so correcting the site never means re-typing the API key — which + * an operator cannot do anyway, since it is never shown back to them. + */ +export async function saveBilling( + client: OpenCompanyClient, + company: string | null, + config: BillingConfig, +): Promise { + return client.put(`${client.scopeFor(company)}/billing/chargebee`, config); +} + +/** Clears every stored Chargebee credential. */ +export async function clearBilling( + client: OpenCompanyClient, + company: string | null, +): Promise { + return client.del(`${client.scopeFor(company)}/billing/chargebee/key`); +} + +/** The non-secret view of a company's PayPal connection (issue #789). */ +export interface PaypalStatus { + /** Whether a client id is stored. Never the id. */ + clientIdConfigured: boolean; + /** Whether a client secret is stored. */ + clientSecretConfigured: boolean; + /** `sandbox` or `live` — which PayPal world the credentials belong to. */ + environment: string; + /** Whether the company's manifest explicitly grants `paypal`. */ + granted: boolean; + /** Whether the `paypal` feature is compiled into the running host. */ + inBuild: boolean; +} + +/** The write-only PayPal save body. Omitted fields keep their stored value. */ +export interface PaypalConfig { + clientId?: string; + clientSecret?: string; + /** `sandbox` or `live`; anything else is stored as `sandbox`. */ + environment?: string; +} + +/** Reads the company's PayPal configuration status. */ +export async function getPaypal( + client: OpenCompanyClient, + company: string | null, +): Promise { + return client.get(`${client.scopeFor(company)}/billing/paypal`); +} + +/** Saves whatever is supplied, and returns the resulting status. */ +export async function savePaypal( + client: OpenCompanyClient, + company: string | null, + config: PaypalConfig, +): Promise { + return client.put(`${client.scopeFor(company)}/billing/paypal`, config); +} + +/** Clears the stored PayPal credentials and resets the environment. */ +export async function clearPaypal( + client: OpenCompanyClient, + company: string | null, +): Promise { + return client.del(`${client.scopeFor(company)}/billing/paypal/key`); +} diff --git a/frontend/src/lib/language.ts b/frontend/src/lib/language.ts index b895ca217..0fb847f85 100644 --- a/frontend/src/lib/language.ts +++ b/frontend/src/lib/language.ts @@ -150,6 +150,10 @@ const TOOL_LABELS: Readonly> = { // does not exist. Two permissions both reading "Use one of its tools" would be // indistinguishable, so the tools that can actually hold one (the catch-all // `Other` group) need real words rather than the generic fallback. + // Billing (issues #788, #789). Only the two that park need words here; the + // read tools never reach an approval card. + chargebee_send_invoice: "Send an invoice to a customer", + chargebee_create_customer: "Add a customer to Chargebee", workspace_write: "Edit a note in its workspace", workspace_read: "Read a note in its workspace", workspace_list: "List its workspace notes", diff --git a/frontend/src/views/BillingView.tsx b/frontend/src/views/BillingView.tsx new file mode 100644 index 000000000..83ea79580 --- /dev/null +++ b/frontend/src/views/BillingView.tsx @@ -0,0 +1,602 @@ +import { useCallback, useEffect, useState } from "react"; +import { Check, Copy, CreditCard, Loader2, TriangleAlert } from "lucide-react"; +import { toast } from "sonner"; + +import { + clearBilling, + clearPaypal, + getBilling, + getPaypal, + saveBilling, + savePaypal, + type BillingStatus, + type PaypalStatus, +} from "@/api/billing"; +import type { OpenCompanyClient } from "@/api/client"; +import { Alert, AlertDescription } from "@/components/ui/alert"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { Card, CardContent } from "@/components/ui/card"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; + +interface Props { + client: OpenCompanyClient; + company: string | null; +} + +/** The message out of a rejected request, whatever it was rejected with. */ +function reason(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +/** + * Settings → Billing: the company's Chargebee connection (issue #788, #527). + * + * # Why four states and not a "Connected ✓" badge + * + * Four separate things can each be missing, and three of them are invisible + * from this form's own fields: + * + * - no key or site — the agent has no billing tools at all + * - no webhook — the tools work, but nobody is told when a customer pays + * - not granted — both credentials stored and STILL nothing reaches an + * agent, because the manifest does not grant `chargebee`. + * The fix is `company.toml`, not this page. + * - not in build — the running host was compiled without the feature, so + * no amount of configuring will do anything. + * + * A single "Connected" badge would be green for the last three and send an + * operator hunting through this form for a problem that is not in it. So each + * is reported on its own terms, and the two that this page cannot fix say where + * the fix actually is. + * + * # Credentials are write-only + * + * The API key and the webhook credential are never returned by the host, so + * they are never rendered. A stored key shows as "Configured", and the input + * stays empty with a placeholder saying that typing replaces it — an input + * pre-filled with dots invites an operator to "correct" a value they cannot + * see, and submitting the dots would store the dots. + */ +export function BillingView({ client, company }: Props) { + const [status, setStatus] = useState(null); + const [loadError, setLoadError] = useState(null); + const [busy, setBusy] = useState(false); + + const [apiKey, setApiKey] = useState(""); + const [site, setSite] = useState(""); + const [webhookSecret, setWebhookSecret] = useState(""); + + const [paypal, setPaypal] = useState(null); + const [paypalError, setPaypalError] = useState(null); + const [clientId, setClientId] = useState(""); + const [clientSecret, setClientSecret] = useState(""); + const [environment, setEnvironment] = useState("sandbox"); + + // Every piece of state here belongs to ONE company: the status badges, the + // site box, and — the dangerous ones — the typed-but-unsaved API key, webhook + // secret and PayPal credentials. `SettingsSection` renders this with + // `key={company}`, so a company switch remounts rather than re-running this + // against carried-over state. + // + // That key is load-bearing, not cosmetic. Clearing fields by hand here fixed + // only the ones somebody remembered, and left an operator who typed a key for + // one company, switched, and pressed Save writing that credential into the + // other company's secret store. It also makes `company` constant for this + // instance's lifetime, so a slow response from a previous company cannot land + // on a later one's view — there is no later one to land on. + // Chargebee and PayPal are unrelated integrations, and they are loaded + // independently for that reason. Under `Promise.all` a single rejection took + // the whole page to an error card, so a PayPal read that failed — a host built + // without the feature answering oddly, a slow store, one bad request — left an + // operator unable to reach the Chargebee form at all, for a problem that had + // nothing to do with Chargebee. Each side now reports its own failure and the + // other still renders. + const load = useCallback(async () => { + const [billing, pp] = await Promise.allSettled([ + getBilling(client, company), + getPaypal(client, company), + ]); + + if (billing.status === "fulfilled") { + setStatus(billing.value); + setLoadError(null); + // Seed the site box with what is stored — it is the one non-secret field, + // and an operator correcting a typo should not have to retype it. + setSite(billing.value.site ?? ""); + } else { + setLoadError(reason(billing.reason)); + } + + if (pp.status === "fulfilled") { + setPaypal(pp.value); + setPaypalError(null); + setEnvironment(pp.value.environment || "sandbox"); + } else { + // Not fatal to the page: the PayPal card says so where the PayPal card is, + // and the Chargebee half above is untouched. + setPaypal(null); + setPaypalError(reason(pp.reason)); + } + }, [client, company]); + + useEffect(() => { + void load(); + }, [load]); + + async function onSave() { + // Send only what was actually entered. The host treats this as a patch, so + // an untouched key keeps its stored value rather than being cleared. + const body: Record = {}; + if (apiKey.trim()) body.apiKey = apiKey.trim(); + if (site.trim() && site.trim() !== (status?.site ?? "")) + body.site = site.trim(); + if (webhookSecret.trim()) body.webhookSecret = webhookSecret.trim(); + + if (Object.keys(body).length === 0) { + toast.info("Nothing to save — fill in a field first."); + return; + } + + setBusy(true); + try { + const next = await saveBilling(client, company, body); + setStatus(next); + // Clear the secret inputs on success: leaving a key sitting in a form + // field after it has been stored is one stray screen-share from a leak. + setApiKey(""); + setWebhookSecret(""); + toast.success("Chargebee settings saved."); + } catch (err) { + toast.error( + err instanceof Error + ? err.message + : "Could not save Chargebee settings.", + ); + } finally { + setBusy(false); + } + } + + async function onClear() { + setBusy(true); + try { + const next = await clearBilling(client, company); + setStatus(next); + setApiKey(""); + setWebhookSecret(""); + setSite(""); + toast.success("Chargebee credentials cleared."); + } catch (err) { + toast.error( + err instanceof Error ? err.message : "Could not clear the credentials.", + ); + } finally { + setBusy(false); + } + } + + async function onSavePaypal() { + const body: Record = {}; + if (clientId.trim()) body.clientId = clientId.trim(); + if (clientSecret.trim()) body.clientSecret = clientSecret.trim(); + if (environment !== (paypal?.environment ?? "sandbox")) + body.environment = environment; + + if (Object.keys(body).length === 0) { + toast.info("Nothing to save — fill in a field first."); + return; + } + setBusy(true); + try { + const next = await savePaypal(client, company, body); + setPaypal(next); + setClientId(""); + setClientSecret(""); + toast.success("PayPal settings saved."); + } catch (err) { + toast.error( + err instanceof Error ? err.message : "Could not save PayPal settings.", + ); + } finally { + setBusy(false); + } + } + + async function onClearPaypal() { + setBusy(true); + try { + const next = await clearPaypal(client, company); + setPaypal(next); + setClientId(""); + setClientSecret(""); + setEnvironment(next.environment || "sandbox"); + toast.success("PayPal disconnected."); + } catch (err) { + toast.error( + err instanceof Error ? err.message : "Could not disconnect PayPal.", + ); + } finally { + setBusy(false); + } + } + + if (loadError) { + return ( +
+ + + + Could not load billing settings: {loadError} + + +
+ ); + } + + if (!status) { + return ( +
+ Loading billing… +
+ ); + } + + const connected = status.apiKeyConfigured && !!status.site; + const paypalConnected = + !!paypal?.clientIdConfigured && !!paypal?.clientSecretConfigured; + + return ( + // `flex-1 overflow-y-auto` is load-bearing, not cosmetic: without it this + // page is clipped at the viewport and the PayPal card below cannot be + // scrolled to at all. Matches McpServersView, its nearest sibling. +
+
+
+

+ Billing +

+

+ Connect Chargebee so your teammates can raise invoices and answer + questions about who has paid, and PayPal so they can report the + wallet — without leaving this app. +

+
+ + {/* The two problems this form cannot fix, said before the form so an + operator does not fill it in and wonder why nothing happened. */} + {!status.inBuild ? ( + + + + This host was built without Chargebee support, so these settings + will be stored and have no effect. Rebuild with the{" "} + chargebee feature. + + + ) : null} + + {status.inBuild && !status.granted ? ( + + + + This company does not grant chargebee, so billing + tools will not reach any teammate even once these credentials are + saved. Add chargebee to [tools].allow in + the company’s manifest — it cannot be fixed from this page. + + + ) : null} + + + +
+
+

Chargebee

+

+ {connected + ? `Connected to ${status.site}.chargebee.com` + : "Not connected yet."} +

+
+ {connected ? ( + + Connected + + ) : null} +
+ +
+
+ + setSite(e.target.value)} + /> +

+ The part before .chargebee.com. Pasting the full + URL is fine. +

+
+ +
+ + setApiKey(e.target.value)} + /> +

+ {status.apiKeyConfigured + ? "Stored. It is never shown again." + : "Stored write-only; it is never shown again."} +

+
+
+ +
+ + {status.apiKeyConfigured || status.webhookConfigured ? ( + + ) : null} +
+ + + + + +
+

Payment notifications

+

+ Optional. Without this, invoicing still works — nobody is just + told when a customer pays. +

+
+ +
+ + {status.webhookUrl ? ( +
+ + +
+ ) : ( + // Deliberately not a disabled box showing a loopback address: + // Chargebee cannot deliver to one, and showing it would send an + // operator to configure a webhook that silently never arrives. +

+ This host has no public URL, so Chargebee cannot reach it. Set{" "} + OPENCOMPANY_PUBLIC_URL to an https address to get + a webhook URL. +

+ )} +
+ +
+ + setWebhookSecret(e.target.value)} + /> +

+ Invent a username and password, save them here as{" "} + username:password, then set the same pair in + Chargebee under{" "} + Protect webhook URL with basic authentication. + Subscribe to payment_succeeded and{" "} + payment_failed. +

+
+ + + + {/* PayPal (issue #789). A form, not a "Connect PayPal" button: these + tools read the company's OWN wallet, so there is no third party for an + OAuth popup to ask permission of. */} + + +
+
+

PayPal

+

+ Lets your teammates report the wallet balance and recent + transactions. Invoice payments already route through PayPal + when you set it as a payment method inside Chargebee — that + needs nothing here. +

+
+ {paypalConnected ? ( + + Connected + + ) : null} +
+ + {/* PayPal failed to load while Chargebee did. Said here rather than + as a page-level error: the Chargebee form above is unaffected and + still usable, which is the whole reason the two load apart. The + form below is hidden because saving against an unknown state would + be guessing — there is no `environment` to compare against, so a + Save would post fields the operator did not change. */} + {paypalError ? ( + + + + Could not load the PayPal connection: {paypalError} + + + ) : null} + + {paypal && !paypal.inBuild ? ( + + + + This host was built without PayPal support, so these settings + will be stored and have no effect. + + + ) : null} + + {paypal?.inBuild && !paypal.granted ? ( + + + + This company does not grant paypal, so wallet + tools will not reach any teammate. Add paypal to{" "} + [tools].allow in the company’s manifest. + + + ) : null} + + {paypalError ? null : ( + <> +
+
+ + setClientId(e.target.value)} + /> +
+
+ + setClientSecret(e.target.value)} + /> +
+
+ +
+ + +

+ Sandbox and live credentials are not interchangeable. + Picking the wrong one fails with “invalid + client”, which reads like a typo. +

+
+ +
+ + {paypal?.clientIdConfigured || + paypal?.clientSecretConfigured ? ( + + ) : null} +
+ + )} + + +
+
+ ); +} diff --git a/frontend/src/views/SettingsSection.tsx b/frontend/src/views/SettingsSection.tsx index 063a0c909..4da183abd 100644 --- a/frontend/src/views/SettingsSection.tsx +++ b/frontend/src/views/SettingsSection.tsx @@ -1,9 +1,19 @@ import { lazy, Suspense } from "react"; -import { Blocks, ChartColumnBig, Plug, type LucideIcon, Settings2, Sparkles, UserCog } from "lucide-react"; +import { + Blocks, + ChartColumnBig, + CreditCard, + Plug, + type LucideIcon, + Settings2, + Sparkles, + UserCog, +} from "lucide-react"; import type { OpenCompanyClient } from "@/api/client"; import type { CompanyFeed } from "@/hooks/use-company"; import { cn } from "@/lib/utils"; +import { BillingView } from "@/views/BillingView"; import { ConnectionsView } from "@/views/ConnectionsView"; import { McpServersView } from "@/views/McpServersView"; import { PeopleView } from "@/views/PeopleView"; @@ -19,6 +29,10 @@ export const SETTINGS_PAGES = [ { id: "people", label: "People", icon: UserCog, hint: "Who can sign in, and as what" }, { id: "connections", label: "Connections", icon: Plug, hint: "Third-party accounts" }, { id: "mcp", label: "MCP Servers", icon: Blocks, hint: "Tool servers and their tools" }, + // Sits beside Connections rather than inside it: an operator looking for + // "where do I put my Chargebee key" searches for billing, not for a + // third-party-accounts drawer. + { id: "billing", label: "Billing", icon: CreditCard, hint: "Invoicing through Chargebee" }, // "What this company knows how to do" read as capability the company performs // — the implication issue #569 exists to remove, set here *before* the tab // gets a chance to correct it. The siblings describe their content; so does @@ -111,6 +125,12 @@ export function SettingsSection({ client, company, feed, sub, onNavigate, onFlag {page === "people" && } {page === "connections" && } {page === "mcp" && } + {/* `key` remounts on a company switch, which is what keeps one + company's typed-but-unsaved credentials out of another's Save. See + the note above `load` in BillingView. */} + {page === "billing" && ( + + )} {page === "skills" && } {page === "usage" && ( `/api/v1/companies/${company}`, + get: async (path: string) => + path.endsWith("/billing/paypal") + ? { + clientIdConfigured: false, + clientSecretConfigured: false, + environment: "sandbox", + granted: true, + inBuild: true, + } + : { + apiKeyConfigured: false, + site: null, + webhookConfigured: false, + webhookUrl: null, + granted: true, + inBuild: true, + }, + } as unknown as OpenCompanyClient; +} + +let container: HTMLDivElement; +let root: Root; + +async function showBilling(company: string) { + await act(async () => { + root.render( + createElement(SettingsSection, { + client: clientFor(company), + company, + feed: { messages: [] } as unknown as CompanyFeed, + sub: "billing", + onNavigate: () => {}, + onFlag: () => {}, + }), + ); + }); +} + +function apiKeyBox(): HTMLInputElement { + const box = container.querySelector('[data-testid="billing-api-key"]'); + if (!box) throw new Error("the API key input is not on the page"); + return box; +} + +/** Types into the field the way an operator does, so React's state updates. */ +async function type(box: HTMLInputElement, value: string) { + await act(async () => { + const setter = Object.getOwnPropertyDescriptor( + window.HTMLInputElement.prototype, + "value", + )?.set; + setter?.call(box, value); + box.dispatchEvent(new Event("input", { bubbles: true })); + }); +} + +beforeEach(() => { + (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); +}); + +describe("billing settings across a company switch", () => { + it("drops a typed-but-unsaved credential when the company changes", async () => { + await showBilling("acme"); + await type(apiKeyBox(), "cb_live_for_acme"); + expect(apiKeyBox().value).toBe("cb_live_for_acme"); + + // The operator switches company without saving. + await showBilling("globex"); + + // The key must NOT still be sitting in the box, where the next Save would + // send it to globex. + expect(apiKeyBox().value).toBe(""); + }); + + it("drops it again on a switch back, not just the first time", async () => { + // A `key` that only changed once — or a clear that ran on mount only — + // would pass the test above and fail this one. + await showBilling("acme"); + await type(apiKeyBox(), "cb_live_for_acme"); + await showBilling("globex"); + await type(apiKeyBox(), "cb_live_for_globex"); + expect(apiKeyBox().value).toBe("cb_live_for_globex"); + + await showBilling("acme"); + expect(apiKeyBox().value).toBe(""); + }); +}); diff --git a/frontend/test/unit/billing-view-branches.test.ts b/frontend/test/unit/billing-view-branches.test.ts new file mode 100644 index 000000000..21b3dd92c --- /dev/null +++ b/frontend/test/unit/billing-view-branches.test.ts @@ -0,0 +1,228 @@ +// @vitest-environment jsdom + +import { act, createElement } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { OpenCompanyClient } from "@/api/client"; +import { BillingView } from "@/views/BillingView"; + +/** + * The conditional surfaces of `BillingView`, which are where its whole job is. + * + * The view's own doc comment explains why it reports four failure modes + * separately rather than as one "Connected" badge: a single green tick is right + * for one of them and actively misleading for the other three, sending an + * operator hunting through a form for a problem that is not in it. That is a + * behaviour, and none of it was exercised — the existing suite covers only the + * company-switch remount. A regression that collapsed the not-granted alert into + * the not-in-build one, or that dropped the connected badge, would have shipped + * green. + * + * The one that most needed a test is `load`: Chargebee and PayPal are unrelated + * integrations loaded together, and under `Promise.all` a PayPal failure blanked + * the Chargebee form too. + */ + +const CHARGEBEE_OK = { + apiKeyConfigured: true, + site: "acme-test", + webhookConfigured: true, + webhookUrl: "https://oc.example/hooks/acme/chargebee", + granted: true, + inBuild: true, +}; + +const PAYPAL_OK = { + clientIdConfigured: true, + clientSecretConfigured: true, + environment: "sandbox", + granted: true, + inBuild: true, +}; + +/** A client answering each billing read with a value or a rejection. */ +function clientWith(answers: { + chargebee?: unknown | Error; + paypal?: unknown | Error; +}): OpenCompanyClient { + const answer = (value: unknown) => + value instanceof Error ? Promise.reject(value) : Promise.resolve(value); + return { + scopeFor: () => "/api/v1/companies/acme", + get: (path: string) => + path.endsWith("/billing/paypal") + ? answer(answers.paypal ?? PAYPAL_OK) + : answer(answers.chargebee ?? CHARGEBEE_OK), + } as unknown as OpenCompanyClient; +} + +let container: HTMLDivElement; +let root: Root; + +async function show(client: OpenCompanyClient) { + await act(async () => { + root.render(createElement(BillingView, { client, company: "acme" })); + }); +} + +function at(testid: string): HTMLElement | null { + return container.querySelector(`[data-testid="${testid}"]`); +} + +beforeEach(() => { + ( + globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean } + ).IS_REACT_ACT_ENVIRONMENT = true; + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); + vi.restoreAllMocks(); +}); + +describe("BillingView load failures", () => { + it("keeps the Chargebee form usable when only PayPal fails to load", async () => { + // The regression this pins: `Promise.all` rejected the pair, so a PayPal + // read that failed sent the whole page to an error card and an operator + // could not reach the Chargebee settings at all — for a problem that had + // nothing to do with Chargebee. + await show(clientWith({ paypal: new Error("paypal is having a moment") })); + + expect(at("billing-load-error")).toBeNull(); + expect(at("billing-view")).not.toBeNull(); + expect(at("billing-site")).not.toBeNull(); + + // And the failure is reported where the failure is. + const failed = at("paypal-load-error"); + expect(failed?.textContent).toContain("paypal is having a moment"); + // The PayPal form is withheld rather than shown against unknown state: with + // no stored `environment` to compare against, a Save would post fields the + // operator never touched. + expect(at("paypal-client-id")).toBeNull(); + }); + + it("shows the page-level error when Chargebee itself fails", async () => { + await show(clientWith({ chargebee: new Error("store unreachable") })); + expect(at("billing-load-error")?.textContent).toContain( + "store unreachable", + ); + }); + + it("renders both halves when both load", async () => { + // The control: the two assertions above are only worth having if the + // ordinary path does NOT produce either error card. + await show(clientWith({})); + expect(at("billing-load-error")).toBeNull(); + expect(at("paypal-load-error")).toBeNull(); + expect(at("billing-connected")).not.toBeNull(); + expect(at("paypal-connected")).not.toBeNull(); + }); +}); + +describe("BillingView status surfaces", () => { + it("names the manifest, not the form, when the company does not grant chargebee", async () => { + // Both credentials stored and still nothing reaches an agent. The remedy is + // `company.toml`, so saying "not connected" here would send the operator + // back through a form that is already correct. + await show(clientWith({ chargebee: { ...CHARGEBEE_OK, granted: false } })); + expect(at("billing-not-granted")?.textContent).toContain("[tools].allow"); + expect(at("billing-not-in-build")).toBeNull(); + }); + + it("says the host lacks the feature, and says only that", async () => { + // Not-in-build outranks not-granted: granting `chargebee` in the manifest + // fixes nothing on a host compiled without it, and showing both alerts + // gives two remedies for one problem. + await show( + clientWith({ + chargebee: { ...CHARGEBEE_OK, granted: false, inBuild: false }, + }), + ); + expect(at("billing-not-in-build")).not.toBeNull(); + expect(at("billing-not-granted")).toBeNull(); + }); + + it("withholds the connected badge until BOTH the key and the site are stored", async () => { + // A key with no site produces requests against no host and a site with no + // key produces unauthenticated ones; either alone is not a connection. + await show(clientWith({ chargebee: { ...CHARGEBEE_OK, site: null } })); + expect(at("billing-connected")).toBeNull(); + + await show( + clientWith({ chargebee: { ...CHARGEBEE_OK, apiKeyConfigured: false } }), + ); + expect(at("billing-connected")).toBeNull(); + }); + + it("explains a missing public URL instead of showing a loopback webhook", async () => { + // Chargebee cannot deliver to a loopback address, so a box containing one + // sends an operator to configure a webhook that silently never arrives. + await show( + clientWith({ chargebee: { ...CHARGEBEE_OK, webhookUrl: null } }), + ); + expect(at("billing-webhook-url")).toBeNull(); + expect(at("billing-no-webhook-url")?.textContent).toContain( + "OPENCOMPANY_PUBLIC_URL", + ); + }); + + it("offers Disconnect only once something is actually stored", async () => { + await show( + clientWith({ + chargebee: { + ...CHARGEBEE_OK, + apiKeyConfigured: false, + webhookConfigured: false, + }, + paypal: { + ...PAYPAL_OK, + clientIdConfigured: false, + clientSecretConfigured: false, + }, + }), + ); + expect(at("billing-clear")).toBeNull(); + expect(at("paypal-clear")).toBeNull(); + + await show(clientWith({})); + expect(at("billing-clear")).not.toBeNull(); + expect(at("paypal-clear")).not.toBeNull(); + }); + + it("reports the PayPal grant and build gaps on their own terms", async () => { + await show(clientWith({ paypal: { ...PAYPAL_OK, granted: false } })); + expect(at("paypal-not-granted")?.textContent).toContain("[tools].allow"); + + await show( + clientWith({ paypal: { ...PAYPAL_OK, granted: false, inBuild: false } }), + ); + expect(at("paypal-not-in-build")).not.toBeNull(); + expect(at("paypal-not-granted")).toBeNull(); + }); + + it("seeds the site box from what is stored and the environment from PayPal", async () => { + // The site is the one non-secret field, so an operator correcting a typo + // should not have to retype it — while the credentials stay empty, because + // an input pre-filled with dots invites somebody to submit the dots. + await show(clientWith({ paypal: { ...PAYPAL_OK, environment: "live" } })); + expect( + container.querySelector('[data-testid="billing-site"]') + ?.value, + ).toBe("acme-test"); + expect( + container.querySelector( + '[data-testid="paypal-environment"]', + )?.value, + ).toBe("live"); + expect( + container.querySelector( + '[data-testid="billing-api-key"]', + )?.value, + ).toBe(""); + }); +}); diff --git a/scripts/ci/feature-lanes.txt b/scripts/ci/feature-lanes.txt index 3b5082e7c..8b3caca53 100644 --- a/scripts/ci/feature-lanes.txt +++ b/scripts/ci/feature-lanes.txt @@ -62,6 +62,8 @@ media | partial | openhuman,mcp,telegram,media | harness::build composio | partial | openhuman,tinycortex,composio | harness::composio::isolation_tests harness::composio::ops_helper_tests harness::composio::live::live_tests server::ops::composio::tests::gated_tests export | partial | export | store::export imap | partial | imap,smtp | server::ops::imap +chargebee | partial | openhuman,tinycortex,chargebee | chargebee +paypal | partial | openhuman,tinycortex,paypal | paypal sidecar | partial | sidecar | brain::sidecar # --- Owed a lane: gated tests exist, and they are currently RED ------------- diff --git a/src/chargebee/api.rs b/src/chargebee/api.rs new file mode 100644 index 000000000..136d7f7d6 --- /dev/null +++ b/src/chargebee/api.rs @@ -0,0 +1,1224 @@ +//! The billing operations issue #788 scopes, expressed against Chargebee's +//! REST API v2 and returning the compact projections in [`super::types`]. +//! +//! This layer knows Chargebee and nothing about agents. The toolbelt bridge in +//! [`crate::harness::chargebee`] wraps each function below as an agent-callable +//! tool; keeping the split means the API shapes can be tested without a harness, +//! and the tool descriptions can change without touching the wire format. +//! +//! Arguments are validated here, before any network call, whenever the check is +//! one Chargebee would also make. That is not redundancy: a local rejection can +//! name the valid set, whereas Chargebee's own error arrives after a round trip +//! and, for the agent, after a turn that looked like it was working. + +use crate::error::{OpenCompanyError, Result}; +use serde_json::Value; + +use super::client::{ChargebeeClient, Form}; +use super::types::{ + CreateCustomerArgs, CustomerSummary, GetInvoiceArgs, InvoiceSummary, ListInvoicesArgs, + SendInvoiceArgs, +}; + +/// Builds the invalid-argument error used for every local validation failure. +pub(crate) fn invalid(message: impl Into) -> OpenCompanyError { + OpenCompanyError::Chargebee { + status: 0, + code: "invalid_arguments".to_string(), + message: message.into(), + } +} + +/// Percent-encodes a path segment. +/// +/// Customer and invoice ids reach the URL path and originate in agent input, so +/// a value containing `/` or `?` would otherwise re-target the request at a +/// different endpoint. +fn urlencode(segment: &str) -> String { + segment + .bytes() + .map(|b| match b { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => { + (b as char).to_string() + } + other => format!("%{other:02X}"), + }) + .collect() +} + +/// Whether a failure is Chargebee refusing `net_term_days` because the site has +/// no payment-terms feature. +/// +/// Matched on the message rather than an `api_error_code`, because Chargebee +/// reports it as a generic `invalid_request`: the specific cause lives only in +/// the prose. Deliberately requires BOTH markers so an unrelated invalid_request +/// mentioning one word is not swallowed. +fn mentions_payment_terms(error: &OpenCompanyError) -> bool { + let text = error.to_string().to_ascii_lowercase(); + text.contains("net_term_days") && text.contains("payment terms") +} + +/// Pulls a required object out of a Chargebee response. +/// +/// Every write below reads one named object (`customer`, `invoice`) out of the +/// reply. Defaulting a missing one to `Null` and projecting it anyway yields a +/// record with an **empty id** that looks successful, and the next call spends +/// it — `customer_id=` on an invoice create, which Chargebee answers with a +/// confusing parameter error far from the real cause. So an absent object is an +/// error here, where it can name what was expected. +fn require<'a>(body: &'a Value, key: &str) -> Result<&'a Value> { + body.get(key).filter(|v| v.is_object()).ok_or_else(|| { + // The body goes to the log, not into the message. This one PARSED, so + // unlike the client's unusable-body case it is a real Chargebee object + // — which is exactly why it must not be quoted back: a reply that was + // missing its `invoice` still carries whatever else Chargebee sent + // about the customer, and this message reaches the model's context and + // the durable transcript. + tracing::warn!( + expected = key, + body = %body.to_string().chars().take(200).collect::(), + "[chargebee] reply carried no `{key}` object" + ); + OpenCompanyError::Chargebee { + status: 0, + code: "unexpected_response".to_string(), + message: format!( + "Chargebee's reply carried no `{key}` object. The reply is in the host log." + ), + } + }) +} + +/// Pulls a required *array* out of a Chargebee response, or fails. +/// +/// A successful empty `list` means "no rows" — that is the real answer and +/// stays real. A missing or non-array `list` means the reply's shape moved, +/// and projecting that as an empty result would be a confident false negative +/// about a billing system (an agent answering "no invoices" to a site that may +/// hold any number of them). `paypal::api::list_transactions` makes the same +/// call for the same reason. +fn require_array<'a>(body: &'a Value, key: &str) -> Result<&'a Vec> { + body.get(key).and_then(Value::as_array).ok_or_else(|| { + tracing::warn!( + expected = key, + body = %body.to_string().chars().take(200).collect::(), + "[chargebee] reply carried no `{key}` array" + ); + OpenCompanyError::Chargebee { + status: 0, + code: "unexpected_response".to_string(), + message: format!( + "Chargebee's reply carried no `{key}` array. The reply is in the host log." + ), + } + }) +} + +/// Projects Chargebee's invoice object onto [`InvoiceSummary`]. +fn summarize_invoice(invoice: &Value, payment_url: Option) -> InvoiceSummary { + let num = |key: &str| invoice.get(key).and_then(Value::as_i64).unwrap_or(0); + InvoiceSummary { + id: invoice + .get("id") + .and_then(Value::as_str) + .unwrap_or_default() + .to_string(), + customer_id: invoice + .get("customer_id") + .and_then(Value::as_str) + .unwrap_or_default() + .to_string(), + status: invoice + .get("status") + .and_then(Value::as_str) + .unwrap_or("unknown") + .to_string(), + currency_code: invoice + .get("currency_code") + .and_then(Value::as_str) + .unwrap_or_default() + .to_string(), + total_in_minor_units: num("total"), + amount_due_in_minor_units: num("amount_due"), + amount_paid_in_minor_units: num("amount_paid"), + due_date: invoice.get("due_date").and_then(Value::as_i64), + line_items: invoice + .get("line_items") + .and_then(Value::as_array) + .map(|items| { + items + .iter() + .filter_map(|li| li.get("description").and_then(Value::as_str)) + .map(str::to_string) + .collect() + }) + .unwrap_or_default(), + payment_url, + // Only `send_invoice` can observe a replay; a fetched or listed invoice + // is never one. + replayed_earlier_invoice: false, + } +} + +/// Projects Chargebee's customer object onto [`CustomerSummary`]. +fn summarize_customer(customer: &Value) -> CustomerSummary { + let text = |key: &str| { + customer + .get(key) + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + .map(str::to_string) + }; + let name = match (text("first_name"), text("last_name")) { + (Some(first), Some(last)) => Some(format!("{first} {last}")), + (Some(one), None) | (None, Some(one)) => Some(one), + (None, None) => None, + }; + CustomerSummary { + id: customer + .get("id") + .and_then(Value::as_str) + .unwrap_or_default() + .to_string(), + email: text("email"), + name, + company: text("company"), + } +} + +/// Looks a customer up by email, returning `None` when no record matches. +pub async fn get_customer( + client: &ChargebeeClient, + email: &str, +) -> Result> { + let email = email.trim(); + if email.is_empty() { + return Err(invalid("`email` is required")); + } + let mut query = Form::new(); + // Chargebee filters take an operator suffix: a bare `email=` is IGNORED + // rather than rejected, which would return an unrelated customer as if it + // were a match — the worst possible failure for a tool that decides whether + // to create one. + query.push("email[is]", email); + query.push("limit", "1"); + let body = client.get("/customers", &query).await?; + Ok(body + .get("list") + .and_then(Value::as_array) + .and_then(|rows| rows.first()) + .and_then(|row| row.get("customer")) + .map(summarize_customer)) +} + +/// Creates a customer. +pub async fn create_customer( + client: &ChargebeeClient, + args: CreateCustomerArgs, +) -> Result { + let email = args.email.trim(); + if email.is_empty() { + return Err(invalid("`email` is required")); + } + let mut form = Form::new(); + form.push("email", email); + if let Some(name) = args + .name + .as_deref() + .map(str::trim) + .filter(|s| !s.is_empty()) + { + // Chargebee has no single `name` field. Splitting on the first space + // keeps "Alan" whole and "Ada Byron" correct; a middle name lands in + // `last_name`, which is wrong in a way nobody is harmed by. + match name.split_once(' ') { + Some((first, last)) => { + form.push("first_name", first); + form.push("last_name", last); + } + None => form.push("first_name", name), + } + } + form.push_opt("company", args.company); + let body = client.post_form("/customers", &form, None).await?; + Ok(summarize_customer(require(&body, "customer")?)) +} + +/// Returns the customer for `email`, creating one when no record matches. +async fn resolve_or_create_customer( + client: &ChargebeeClient, + email: &str, + name: Option, +) -> Result { + if let Some(found) = get_customer(client, email).await? { + return Ok(found); + } + create_customer( + client, + CreateCustomerArgs { + email: email.to_string(), + name, + company: None, + }, + ) + .await +} + +/// Raises a hosted page where the customer can settle what they owe. +/// +/// Best-effort by design: this is a second call after the invoice already +/// exists, and a site without a configured gateway (or without the hosted-page +/// feature) refuses it. Failing the whole tool at that point would report "no +/// invoice" for an invoice that was in fact created — the worst answer +/// available. So a failure logs and yields `None`, and the caller says the +/// invoice was raised without a link. +async fn payment_url( + client: &ChargebeeClient, + customer_id: &str, + currency: &str, +) -> Option { + let mut form = Form::new(); + form.push("customer[id]", customer_id); + form.push("currency_code", currency); + match client + .post_form("/hosted_pages/collect_now", &form, None) + .await + { + Ok(body) => body + .get("hosted_page") + .and_then(|p| p.get("url")) + .and_then(Value::as_str) + .map(str::to_string), + Err(e) => { + tracing::warn!(%customer_id, error = %e, "[chargebee] could not raise a payment link"); + None + } + } +} + +/// Derives an idempotency key from the request itself. +/// +/// # Why a key is always sent, even when the caller supplied none +/// +/// The runtime's at-most-once guard covers **approval replay**: an approved +/// effect is recorded executed before it is performed, so re-approving does not +/// re-send. It does not cover **transport retry**, which is the failure that +/// actually duplicates an invoice — the request reaches Chargebee, the response +/// is lost to a timeout, the tool reports failure, and the agent (or an +/// operator reading that failure) sends again. The customer receives two +/// invoices. +/// +/// The key was an optional tool argument, which in practice meant absent: a +/// model has no reason to invent one, and every send observed in testing +/// omitted it. Deriving one from the request body closes that by default. It is +/// deliberately derived from the REQUEST rather than from the approved effect — +/// the effect id is not reachable here, because an approved call is re-issued +/// by the model through the ordinary tool path (`redispatch_granted_call`) +/// rather than executed by the runtime with the effect in scope. +/// +/// The trade this makes is explicit: two byte-identical invoices raised inside +/// Chargebee's key-retention window collapse to one. That is why a replay is +/// reported back rather than passed off as a new invoice — see +/// [`InvoiceSummary::replayed_earlier_invoice`] — and why a caller who means to +/// bill twice can pass a distinct `idempotency_key`. +/// +/// FNV-1a rather than `DefaultHasher`, so the key is stable **as a value**, not +/// merely within one process. `DefaultHasher`'s output is explicitly not +/// guaranteed across Rust releases, which would mean a host upgraded mid-retry +/// — or two hosts of one company behind a load balancer — deriving different +/// keys for the same invoice and billing the customer twice. That is precisely +/// the failure this function exists to prevent, so the hash cannot be one whose +/// stability is a footnote about the toolchain. The field separators keep +/// `("ab","c")` from colliding with `("a","bc")`. +fn derived_idempotency_key(form: &Form) -> String { + const OFFSET: u64 = 0xcbf2_9ce4_8422_2325; + const PRIME: u64 = 0x0000_0100_0000_01b3; + let mut hash = OFFSET; + let mut eat = |bytes: &[u8]| { + for byte in bytes { + hash ^= *byte as u64; + hash = hash.wrapping_mul(PRIME); + } + }; + for (key, value) in form.pairs() { + eat(key.as_bytes()); + eat(b"="); + eat(value.as_bytes()); + eat(b"&"); + } + format!("oc-invoice-{hash:016x}") +} + +/// Creates an invoice for `customer_email`, creating the customer if needed, +/// and returns it with a payment link when one could be raised. +pub async fn send_invoice( + client: &ChargebeeClient, + args: SendInvoiceArgs, +) -> Result { + if args.line_items.is_empty() { + return Err(invalid("`line_items` must contain at least one entry")); + } + let currency = args.currency_code.trim().to_uppercase(); + if currency.is_empty() { + return Err(invalid("`currency_code` is required, e.g. USD")); + } + for (i, line) in args.line_items.iter().enumerate() { + if line.amount_in_minor_units < 1 { + return Err(invalid(format!( + "line_items[{i}].amount_in_minor_units must be at least 1 (amounts are in minor \ + units — $100.00 is 10000)" + ))); + } + } + + let customer = + resolve_or_create_customer(client, args.customer_email.trim(), args.customer_name).await?; + + let mut form = Form::new(); + form.push("customer_id", &customer.id); + form.push("currency_code", ¤cy); + // Unasked-for, and load-bearing. Chargebee's default follows the customer + // record and charges a stored card the moment the invoice exists — verified + // against a live site, which answered `payment_method_not_present`. Two + // reasons to override it: "send an invoice" is not "take a payment", and an + // auto-collected invoice is already paid, which would make the "has Alan + // paid?" flow (#788) answer itself. + form.push("auto_collection", "off"); + form.push_opt("net_term_days", args.due_days); + form.push_opt("invoice_note", args.invoice_note); + for (i, line) in args.line_items.iter().enumerate() { + form.push_indexed("charges", "description", i, &line.description); + form.push_indexed("charges", "amount", i, line.amount_in_minor_units); + } + + let path = "/invoices/create_for_charge_items_and_charges"; + let key = args + .idempotency_key + .clone() + .unwrap_or_else(|| derived_idempotency_key(&form)); + let (body, replayed) = match client.post_form_replayable(path, &form, Some(&key)).await { + Ok(outcome) => outcome, + // `net_term_days` is refused outright by a site that has not enabled + // "Payment Terms for One-Time Invoices" — a per-site feature most test + // sites ship without. Failing the whole invoice over a DUE DATE is the + // wrong trade: the operator asked for an invoice and would rather have + // one without terms than none at all. So the term is dropped and the + // call retried once, and the caller is told in the log. + // + // Narrow on purpose: only this one error, and only when we actually + // sent the field. Anything else propagates untouched. + Err(e) if args.due_days.is_some() && mentions_payment_terms(&e) => { + tracing::warn!( + "[chargebee] this site has not enabled payment terms for one-time invoices; \ + raising the invoice without a due date" + ); + let mut retry = Form::new(); + for (field, value) in form.pairs() { + if field != "net_term_days" { + retry.push(field.clone(), value.clone()); + } + } + // A DIFFERENT key from the first attempt, deliberately. Chargebee + // may have stored that attempt's 400 against its key, and replaying + // a refusal would turn the recovery into the failure it exists to + // avoid. The retry is a genuinely different request — it asks for + // no payment terms — so it gets its own key. A derived key changes + // on its own, since the body changed; a caller-supplied one is + // suffixed rather than reused. + let retry_key = match &args.idempotency_key { + Some(supplied) => format!("{supplied}-no-terms"), + None => derived_idempotency_key(&retry), + }; + client + .post_form_replayable(path, &retry, Some(&retry_key)) + .await? + } + Err(e) => return Err(e), + }; + let invoice = require(&body, "invoice")?.clone(); + let url = payment_url(client, &customer.id, ¤cy).await; + let mut summary = summarize_invoice(&invoice, url); + if replayed { + // Chargebee returned an earlier invoice verbatim, so nothing was + // raised. Reported rather than swallowed: for a retry this is the + // outcome you want, and for a deliberate second charge it is the one + // fact that distinguishes "billed twice" from "billed once". + tracing::warn!( + invoice_id = %summary.id, + "[chargebee] send_invoice replayed an earlier invoice for this idempotency key" + ); + summary.replayed_earlier_invoice = true; + } + Ok(summary) +} + +/// Fetches one invoice by id. +pub async fn get_invoice(client: &ChargebeeClient, args: GetInvoiceArgs) -> Result { + let id = args.invoice_id.trim(); + if id.is_empty() { + return Err(invalid("`invoice_id` is required")); + } + let body = client + .get(&format!("/invoices/{}", urlencode(id)), &Form::new()) + .await?; + Ok(summarize_invoice(require(&body, "invoice")?, None)) +} + +/// Lists invoices, optionally narrowed to one customer and/or status. +pub async fn list_invoices( + client: &ChargebeeClient, + args: ListInvoicesArgs, +) -> Result> { + let mut query = Form::new(); + if let Some(email) = args + .customer_email + .as_deref() + .map(str::trim) + .filter(|s| !s.is_empty()) + { + // An email that matches nobody must return "no invoices", not "every + // invoice on the site" — which is what dropping an unresolvable filter + // would do. + let Some(customer) = get_customer(client, email).await? else { + return Ok(Vec::new()); + }; + query.push("customer_id[is]", customer.id); + } + query.push_opt("status[is]", args.status); + query.push_opt("limit", args.limit); + + let body = client.get("/invoices", &query).await?; + let rows = require_array(&body, "list")?; + Ok(rows + .iter() + .filter_map(|row| row.get("invoice")) + .map(|invoice| summarize_invoice(invoice, None)) + .collect()) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn an_invoice_summary_keeps_the_facts_an_operator_asked_about() { + let raw = json!({ + "id": "inv_1", "customer_id": "cus_1", "status": "payment_due", + "currency_code": "USD", "total": 10000, "amount_due": 10000, + "amount_paid": 0, "due_date": 1786603009, + "line_items": [{"description": "Consulting", "amount": 10000}], + // Noise the projection must drop rather than hand to a model. + "linked_payments": [], "site_details_at_creation": {"timezone": "UTC"} + }); + let s = summarize_invoice(&raw, Some("https://pay.example".to_string())); + assert_eq!(s.id, "inv_1"); + assert_eq!(s.status, "payment_due"); + assert_eq!(s.total_in_minor_units, 10_000); + assert_eq!(s.amount_due_in_minor_units, 10_000); + assert_eq!(s.line_items, vec!["Consulting".to_string()]); + assert_eq!(s.payment_url.as_deref(), Some("https://pay.example")); + } + + #[test] + fn a_missing_field_does_not_panic_the_projection() { + // Chargebee omits `due_date` on some invoice shapes, and a summary that + // panicked on one would take the whole turn with it. + let s = summarize_invoice(&json!({"id": "inv_2"}), None); + assert_eq!(s.id, "inv_2"); + assert_eq!(s.status, "unknown"); + assert_eq!(s.due_date, None); + assert!(s.line_items.is_empty()); + } + + #[test] + fn a_full_name_splits_into_chargebee_first_and_last() { + let one = summarize_customer(&json!({"id": "c1", "first_name": "Alan"})); + assert_eq!(one.name.as_deref(), Some("Alan")); + let two = summarize_customer( + &json!({"id": "c2", "first_name": "Ada", "last_name": "Byron", "email": "a@b.test"}), + ); + assert_eq!(two.name.as_deref(), Some("Ada Byron")); + assert_eq!(two.email.as_deref(), Some("a@b.test")); + } + + #[test] + fn ids_are_percent_encoded_so_they_cannot_retarget_the_path() { + assert_eq!(urlencode("inv_123"), "inv_123"); + assert_eq!(urlencode("../customers/x"), "..%2Fcustomers%2Fx"); + } + + // ---- Wire-level tests ------------------------------------------------- + // + // These drive a stub over a real socket rather than stopping at argument + // validation. The wire format is where this module is most likely to be + // wrong — form-encoded rather than JSON, `charges[amount][0]` nesting, + // `email[is]` rather than `email` — and none of it is exercised by a test + // that never builds a request. Every shape asserted below was also checked + // against a live Chargebee site. + + use crate::chargebee::types::{ChargeLine, ChargebeeConfig}; + use std::sync::{Arc, Mutex}; + + /// A canned reply: `" "`, status, JSON body. + type Route = (&'static str, u16, &'static str); + /// The stub's shared state: what it has seen, and what to answer with. + type StubState = (Arc>>, Arc>); + + /// One request the stub saw. + #[derive(Clone, Debug)] + struct Seen { + method: String, + path: String, + query: String, + body: String, + /// The `chargebee-idempotency-key` header, when one was sent. + idempotency: Option, + } + + /// Serves canned responses by path prefix and records every request. + /// + /// Keyed on `" "` because `send_invoice` is three + /// calls in a row (customer lookup, invoice create, payment link) and two of + /// them share the `/customers` path — a route table keyed on path alone + /// answers the create with the lookup's body, which is exactly how the + /// fabricated-empty-id bug surfaced. + /// + /// Listing the SAME prefix more than once makes it answer differently per + /// attempt: the Nth request matching a prefix gets that prefix's Nth entry, + /// clamped to the last. That is what lets a test drive a failure and its + /// retry through one route table; a prefix listed once behaves as before. + async fn stub(routes: Vec, call: F) -> (Result, Vec) + where + F: FnOnce(ChargebeeClient) -> Fut, + Fut: std::future::Future>, + { + use axum::extract::State; + let seen: Arc>> = Arc::new(Mutex::new(Vec::new())); + let routes = Arc::new(routes); + + let handler = move |State((seen, routes)): State, + method: axum::http::Method, + uri: axum::http::Uri, + headers: axum::http::HeaderMap, + body: String| async move { + let path = format!("{method} {}", uri.path()); + let attempt = { + let mut log = seen.lock().expect("lock"); + let attempt = log + .iter() + .filter(|s| format!("{} {}", s.method, s.path) == path) + .count(); + log.push(Seen { + method: method.to_string(), + path: uri.path().to_string(), + query: uri.query().unwrap_or_default().to_string(), + body, + idempotency: headers + .get("chargebee-idempotency-key") + .and_then(|v| v.to_str().ok()) + .map(str::to_string), + }); + attempt + }; + let matching: Vec<&Route> = routes + .iter() + .filter(|(prefix, _, _)| path.contains(prefix)) + .collect(); + let (status, payload) = matching + .get(attempt.min(matching.len().saturating_sub(1))) + .map(|(_, s, b)| (*s, *b)) + .unwrap_or((404, "{}")); + let mut out = axum::http::HeaderMap::new(); + out.insert("content-type", "application/json".parse().expect("header")); + // Test affordance: an idempotency key beginning `replay-` makes the + // stub answer the way Chargebee answers a replayed request, so the + // replay path can be driven end to end without a second live send. + if seen + .lock() + .expect("lock") + .last() + .and_then(|s| s.idempotency.as_deref()) + .is_some_and(|key| key.starts_with("replay-")) + { + out.insert( + "chargebee-idempotency-replayed", + "true".parse().expect("header"), + ); + } + ( + axum::http::StatusCode::from_u16(status).expect("status"), + out, + payload, + ) + }; + + let app = axum::Router::new() + .fallback(axum::routing::any(handler)) + .with_state((seen.clone(), routes)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("addr"); + let server = tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let client = ChargebeeClient::with_base_url( + ChargebeeConfig { + site: "test".to_string(), + api_key: "cb_key".to_string(), + }, + format!("http://{addr}"), + ) + .expect("client builds"); + let out = call(client).await; + server.abort(); + let requests = seen.lock().expect("lock").clone(); + (out, requests) + } + + fn line(desc: &str, minor: i64) -> ChargeLine { + ChargeLine { + description: desc.to_string(), + amount_in_minor_units: minor, + } + } + + const NO_CUSTOMER: &str = r#"{"list":[]}"#; + const ONE_CUSTOMER: &str = + r#"{"list":[{"customer":{"id":"cus_1","email":"alan@tinyhumans.ai"}}]}"#; + const CREATED_CUSTOMER: &str = r#"{"customer":{"id":"cus_new","email":"alan@tinyhumans.ai"}}"#; + const CREATED_INVOICE: &str = r#"{"invoice":{"id":"inv_1","customer_id":"cus_new","status":"payment_due","currency_code":"USD","total":10000,"amount_due":10000,"amount_paid":0,"line_items":[{"description":"Consulting"}]}}"#; + const HOSTED_PAGE: &str = + r#"{"hosted_page":{"url":"https://acme.chargebee.com/pages/v3/abc"}}"#; + + #[tokio::test] + async fn an_unknown_customer_is_created_before_invoicing() { + // TC-05: the operator names an email, not an internal id. + let (result, seen) = stub( + vec![ + ("GET /customers", 200, NO_CUSTOMER), + ("POST /customers", 200, CREATED_CUSTOMER), + ( + "POST /invoices/create_for_charge_items_and_charges", + 200, + CREATED_INVOICE, + ), + ("POST /hosted_pages/collect_now", 200, HOSTED_PAGE), + ], + |client| async move { + send_invoice( + &client, + SendInvoiceArgs { + customer_email: "alan@tinyhumans.ai".to_string(), + customer_name: Some("Alan Turing".to_string()), + currency_code: "usd".to_string(), + line_items: vec![line("Consulting", 10_000)], + due_days: Some(7), + invoice_note: None, + idempotency_key: None, + }, + ) + .await + }, + ) + .await; + + let invoice = result.expect("invoice is created"); + assert_eq!(invoice.status, "payment_due"); + assert_eq!(invoice.total_in_minor_units, 10_000); + assert_eq!( + invoice.payment_url.as_deref(), + Some("https://acme.chargebee.com/pages/v3/abc") + ); + + // The exact sequence matters: look up, then create, then invoice the id + // the create returned. Asserting it here is what catches a reordering + // that would invoice against an id nothing had produced yet. + let calls: Vec<(&str, &str)> = seen + .iter() + .map(|r| (r.method.as_str(), r.path.as_str())) + .collect(); + assert_eq!( + calls, + vec![ + ("GET", "/customers"), + ("POST", "/customers"), + ("POST", "/invoices/create_for_charge_items_and_charges"), + ("POST", "/hosted_pages/collect_now"), + ] + ); + + // The lookup is filtered with the operator suffix — a bare `email=` is + // ignored by Chargebee and would match the wrong customer. + assert!(seen[0].query.contains("email%5Bis%5D="), "{:?}", seen[0]); + // Then the create, splitting the display name across Chargebee's fields. + assert!(seen[1].body.contains("first_name=Alan"), "{:?}", seen[1]); + assert!(seen[1].body.contains("last_name=Turing"), "{:?}", seen[1]); + // Then the invoice, against the id the create returned. + let inv = &seen[2]; + assert!(inv.body.contains("customer_id=cus_new"), "{inv:?}"); + assert!(inv.body.contains("currency_code=USD"), "{inv:?}"); + assert!(inv.body.contains("net_term_days=7"), "{inv:?}"); + assert!( + inv.body.contains("charges%5Bamount%5D%5B0%5D=10000"), + "{inv:?}" + ); + // The guard that a live site taught us: without this Chargebee charges + // a stored card on creation and the invoice is born paid. + assert!(inv.body.contains("auto_collection=off"), "{inv:?}"); + } + + #[tokio::test] + async fn an_existing_customer_is_reused_and_never_renamed() { + let (result, seen) = stub( + vec![ + ("GET /customers", 200, ONE_CUSTOMER), + ( + "POST /invoices/create_for_charge_items_and_charges", + 200, + CREATED_INVOICE, + ), + ("POST /hosted_pages/collect_now", 200, HOSTED_PAGE), + ], + |client| async move { + send_invoice( + &client, + SendInvoiceArgs { + customer_email: "alan@tinyhumans.ai".to_string(), + // Supplied, and must be ignored: invoicing someone is + // not a licence to rewrite their name. + customer_name: Some("Wrong Name".to_string()), + currency_code: "USD".to_string(), + line_items: vec![line("Consulting", 10_000)], + due_days: None, + invoice_note: None, + idempotency_key: None, + }, + ) + .await + }, + ) + .await; + + result.expect("invoice is created"); + assert!( + !seen.iter().any(|r| r.body.contains("Wrong")), + "an existing customer must not be renamed: {seen:?}" + ); + assert!( + seen.iter().any(|r| r.body.contains("customer_id=cus_1")), + "the existing id must be used: {seen:?}" + ); + } + + #[tokio::test] + async fn an_unresolvable_email_returns_no_invoices_rather_than_all_of_them() { + // Dropping an unresolvable filter would list the whole site's invoices + // in answer to "has this stranger paid?". + let (result, seen) = stub( + vec![("GET /customers", 200, NO_CUSTOMER)], + |client| async move { + list_invoices( + &client, + ListInvoicesArgs { + customer_email: Some("nobody@nowhere.test".to_string()), + ..Default::default() + }, + ) + .await + }, + ) + .await; + + assert!(result.expect("lookup succeeds").is_empty()); + assert_eq!( + seen.len(), + 1, + "the invoice list must not be reached: {seen:?}" + ); + } + + #[tokio::test] + async fn a_reply_without_a_list_array_is_an_error_not_an_empty_invoice_list() { + // `{"list":[]}` is a real empty history, but a 2xx body with no `list` + // array means the reply's shape moved — reporting that as "no invoices" + // would be a confident false negative about the billing ledger. + let (result, _seen) = stub( + vec![("GET /invoices", 200, r#"{"site":"acme"}"#)], + |client| async move { list_invoices(&client, ListInvoicesArgs::default()).await }, + ) + .await; + + let err = result.expect_err("a missing `list` is an error"); + assert!(err.to_string().contains("no `list` array"), "{err}"); + } + + #[tokio::test] + async fn an_invoice_survives_a_payment_link_that_cannot_be_raised() { + // A site with no gateway configured refuses the hosted page. Reporting + // "no invoice" for an invoice that exists is the worst answer available. + let (result, _) = stub( + vec![ + ("GET /customers", 200, ONE_CUSTOMER), + ( + "POST /invoices/create_for_charge_items_and_charges", + 200, + CREATED_INVOICE, + ), + ( + "POST /hosted_pages/collect_now", + 400, + r#"{"message":"no gateway","api_error_code":"invalid_request"}"#, + ), + ], + |client| async move { + send_invoice( + &client, + SendInvoiceArgs { + customer_email: "alan@tinyhumans.ai".to_string(), + customer_name: None, + currency_code: "USD".to_string(), + line_items: vec![line("Consulting", 10_000)], + due_days: None, + invoice_note: None, + idempotency_key: None, + }, + ) + .await + }, + ) + .await; + + let invoice = result.expect("the invoice itself still succeeds"); + assert_eq!(invoice.id, "inv_1"); + assert_eq!(invoice.payment_url, None); + } + + #[tokio::test] + async fn a_site_without_payment_terms_still_gets_its_invoice() { + // Chargebee refuses `net_term_days` outright on a site that has not + // enabled payment terms for one-time invoices. Failing the whole + // invoice over a due date is the wrong trade — the operator asked for + // an invoice, and one without terms beats none. + let mut calls = 0; + let (result, seen) = stub( + vec![ + ("GET /customers", 200, ONE_CUSTOMER), + ( + "POST /invoices/create_for_charge_items_and_charges", + 200, + CREATED_INVOICE, + ), + ("POST /hosted_pages/collect_now", 200, HOSTED_PAGE), + ], + |client| async move { + let _ = &mut calls; + send_invoice( + &client, + SendInvoiceArgs { + customer_email: "alan@tinyhumans.ai".to_string(), + customer_name: None, + currency_code: "INR".to_string(), + line_items: vec![line("Consulting", 10_000)], + due_days: Some(7), + invoice_note: None, + idempotency_key: None, + }, + ) + .await + }, + ) + .await; + // The happy path still sends the term when the site accepts it. + result.expect("invoice created"); + assert!( + seen.iter().any(|r| r.body.contains("net_term_days=7")), + "the term must still be sent to a site that accepts it: {seen:?}" + ); + } + + /// Chargebee's own words when the site lacks the feature. + const TERMS_REFUSED: &str = r#"{"api_error_code":"invalid_request","message":"net_term_days : should not be sent as the Payment Terms for One-Time Invoices feature is not enabled"}"#; + + #[tokio::test] + async fn the_payment_terms_refusal_is_retried_without_the_term() { + // The other half of the trade above: when the site actually refuses, + // the invoice is raised anyway, once, without `net_term_days`. + let (result, seen) = stub( + vec![ + ("GET /customers", 200, ONE_CUSTOMER), + // Listed twice — the first attempt is refused, the retry lands. + ( + "POST /invoices/create_for_charge_items_and_charges", + 400, + TERMS_REFUSED, + ), + ( + "POST /invoices/create_for_charge_items_and_charges", + 200, + CREATED_INVOICE, + ), + ("POST /hosted_pages/collect_now", 200, HOSTED_PAGE), + ], + |client| async move { + send_invoice( + &client, + SendInvoiceArgs { + customer_email: "alan@tinyhumans.ai".to_string(), + customer_name: None, + currency_code: "USD".to_string(), + line_items: vec![line("Consulting", 10_000)], + due_days: Some(7), + invoice_note: None, + idempotency_key: None, + }, + ) + .await + }, + ) + .await; + + let invoice = result.expect("the invoice survives a site without payment terms"); + assert_eq!(invoice.id, "inv_1"); + + let creates: Vec<&Seen> = seen + .iter() + .filter(|r| r.path.contains("create_for_charge_items_and_charges")) + .collect(); + assert_eq!(creates.len(), 2, "one refusal, one retry: {seen:?}"); + assert!( + creates[0].body.contains("net_term_days=7"), + "the first attempt asks for the term: {:?}", + creates[0] + ); + assert!( + !creates[1].body.contains("net_term_days"), + "the retry must drop it: {:?}", + creates[1] + ); + // The rest of the invoice is unchanged — a retry that also lost the + // amount would be worse than the failure it replaces. + assert!(creates[1].body.contains("charges%5Bamount%5D%5B0%5D=10000")); + // And it carries a DIFFERENT key: Chargebee may have stored the 400 + // against the first one, and replaying a refusal would defeat the + // retry entirely. + let first = creates[0].idempotency.as_deref().expect("first key"); + let retry = creates[1].idempotency.as_deref().expect("retry key"); + assert_ne!(first, retry, "the retry needs its own key: {seen:?}"); + } + + #[tokio::test] + async fn every_send_carries_an_idempotency_key_even_when_none_was_supplied() { + // The model has no reason to invent one, so every send observed in + // testing omitted it — leaving a lost response and a resend to bill the + // customer twice. + let (result, seen) = stub( + vec![ + ("GET /customers", 200, ONE_CUSTOMER), + ( + "POST /invoices/create_for_charge_items_and_charges", + 200, + CREATED_INVOICE, + ), + ("POST /hosted_pages/collect_now", 200, HOSTED_PAGE), + ], + |client| async move { + send_invoice( + &client, + SendInvoiceArgs { + customer_email: "alan@tinyhumans.ai".to_string(), + customer_name: None, + currency_code: "USD".to_string(), + line_items: vec![line("Consulting", 10_000)], + due_days: None, + invoice_note: None, + idempotency_key: None, + }, + ) + .await + }, + ) + .await; + let invoice = result.expect("invoice created"); + // Nothing was replayed, so the field stays out of the agent's view. + assert!(!invoice.replayed_earlier_invoice); + let create = seen + .iter() + .find(|r| r.path.contains("create_for_charge_items_and_charges")) + .expect("the invoice was created"); + assert!( + create + .idempotency + .as_deref() + .is_some_and(|key| key.starts_with("oc-invoice-")), + "a key must be derived when the caller supplies none: {create:?}" + ); + } + + #[test] + fn a_derived_key_is_a_fixed_value_not_merely_self_consistent() { + // Pinned as a literal on purpose. A hash that is only stable within one + // process still bills a customer twice when the retry lands on a host + // built from a different toolchain, or on a sibling behind a load + // balancer -- so "same input, same key" has to hold across builds, and + // the only way to assert that is to write the value down. + let mut form = Form::new(); + form.push("customer_id", "cus_1"); + form.push("currency_code", "USD"); + form.push_indexed("charges", "amount", 0, 10_000); + let key = derived_idempotency_key(&form); + assert_eq!(key, "oc-invoice-e989cc10e2e5e7d0", "{key}"); + + // Field boundaries are part of the input: without a separator these two + // hash identically, and two different invoices would share a key -- + // which silently drops the second. + let mut ab_c = Form::new(); + ab_c.push("ab", "c"); + let mut a_bc = Form::new(); + a_bc.push("a", "bc"); + assert_ne!( + derived_idempotency_key(&ab_c), + derived_idempotency_key(&a_bc) + ); + } + + #[tokio::test] + async fn a_derived_key_is_stable_for_the_same_invoice_and_differs_across_invoices() { + // The whole point: a retry of the same send must reuse the key, and a + // different invoice must not collide with it. + async fn key_for(amount: i64) -> String { + let (result, seen) = stub( + vec![ + ("GET /customers", 200, ONE_CUSTOMER), + ( + "POST /invoices/create_for_charge_items_and_charges", + 200, + CREATED_INVOICE, + ), + ("POST /hosted_pages/collect_now", 200, HOSTED_PAGE), + ], + |client| async move { + send_invoice( + &client, + SendInvoiceArgs { + customer_email: "alan@tinyhumans.ai".to_string(), + customer_name: None, + currency_code: "USD".to_string(), + line_items: vec![line("Consulting", amount)], + due_days: None, + invoice_note: None, + idempotency_key: None, + }, + ) + .await + }, + ) + .await; + result.expect("invoice created"); + seen.iter() + .find(|r| r.path.contains("create_for_charge_items_and_charges")) + .and_then(|r| r.idempotency.clone()) + .expect("a key was sent") + } + + assert_eq!(key_for(10_000).await, key_for(10_000).await); + assert_ne!(key_for(10_000).await, key_for(20_000).await); + } + + #[tokio::test] + async fn a_replayed_invoice_says_so_rather_than_reading_as_a_new_one() { + // A replay returns the original invoice verbatim, so without this flag + // a deliberate second charge that was deduped is indistinguishable from + // a successful new invoice — a silent failure to bill. + let (result, _seen) = stub( + vec![ + ("GET /customers", 200, ONE_CUSTOMER), + ( + "POST /invoices/create_for_charge_items_and_charges", + 200, + CREATED_INVOICE, + ), + ("POST /hosted_pages/collect_now", 200, HOSTED_PAGE), + ], + |client| async move { + send_invoice( + &client, + SendInvoiceArgs { + customer_email: "alan@tinyhumans.ai".to_string(), + customer_name: None, + currency_code: "USD".to_string(), + line_items: vec![line("Consulting", 10_000)], + due_days: None, + invoice_note: None, + // The stub answers this the way Chargebee answers a + // replay. + idempotency_key: Some("replay-abc".to_string()), + }, + ) + .await + }, + ) + .await; + + let invoice = result.expect("a replay is still a successful call"); + assert!( + invoice.replayed_earlier_invoice, + "the replay must be reported: {invoice:?}" + ); + let rendered = serde_json::to_string(&invoice).expect("serialises"); + assert!( + rendered.contains("replayed_earlier_invoice"), + "and it must reach the agent: {rendered}" + ); + } + + #[test] + fn only_the_payment_terms_refusal_triggers_the_retry() { + let terms = OpenCompanyError::Chargebee { + status: 400, + code: "invalid_request".to_string(), + message: "net_term_days : should not be sent as the Payment Terms for One-Time \ + Invoices feature is not enabled" + .to_string(), + }; + assert!(mentions_payment_terms(&terms)); + + // An unrelated invalid_request that happens to mention one word must + // NOT be swallowed and silently retried. + let other = OpenCompanyError::Chargebee { + status: 400, + code: "invalid_request".to_string(), + message: "net_term_days must be a positive integer".to_string(), + }; + assert!(!mentions_payment_terms(&other)); + assert!(!mentions_payment_terms(&invalid("something else entirely"))); + } + + #[tokio::test] + async fn dollars_written_as_cents_are_caught_at_the_floor_only() { + // The naming convention carries the real weight; the floor is all a + // guard can check without reading intent. + let (result, seen) = stub(vec![], |client| async move { + send_invoice( + &client, + SendInvoiceArgs { + customer_email: "alan@tinyhumans.ai".to_string(), + customer_name: None, + currency_code: "USD".to_string(), + line_items: vec![line("Consulting", 0)], + due_days: None, + invoice_note: None, + idempotency_key: None, + }, + ) + .await + }) + .await; + + let err = result.expect_err("a zero amount is rejected"); + assert!(err.to_string().contains("at least 1"), "got: {err}"); + assert!(seen.is_empty(), "rejected before any request: {seen:?}"); + } +} diff --git a/src/chargebee/client.rs b/src/chargebee/client.rs new file mode 100644 index 000000000..f95744d43 --- /dev/null +++ b/src/chargebee/client.rs @@ -0,0 +1,451 @@ +//! A thin HTTP client for the Chargebee Billing API v2. +//! +//! Three things about Chargebee's wire format drive the shape of this module, +//! and all three are easy to get wrong from memory (each is checked against +//! `spec/chargebee_api_v2_pc_v2_spec.json` in the `chargebee/openapi` repo): +//! +//! 1. **Writes are `application/x-www-form-urlencoded`, not JSON.** Posting +//! JSON to `/invoices/create_for_charge_items_and_charges` fails with a +//! parameter error, not a content-type error, so the mistake reads as a bad +//! request body. +//! 2. **Nested parameters use bracket-array notation.** A two-line invoice is +//! `charges[description][0]=…&charges[amount][0]=…&charges[description][1]=…` +//! — the index is per-field, not per-object. [`Form::push_indexed`] is the +//! only place that encoding exists. +//! 3. **Auth is HTTP Basic with the API key as the username and an empty +//! password.** Not a bearer token. + +use crate::error::{OpenCompanyError, Result}; +use serde_json::Value; + +use super::types::ChargebeeConfig; + +/// The header Chargebee reads for idempotent replay of a `POST`. +const IDEMPOTENCY_HEADER: &str = "chargebee-idempotency-key"; + +/// The header Chargebee sets when it replayed a stored response instead of +/// performing the request again. +const REPLAYED_HEADER: &str = "chargebee-idempotency-replayed"; + +/// Reports a body that could not be interpreted, WITHOUT putting it in the +/// message. +/// +/// A classified Chargebee error (`api_error_code` plus its own `message`) is +/// something the agent should read — it names a business outcome, and the model +/// can act on it. This function is the opposite case by construction: it runs +/// when the body could not be parsed, so its contents are unknown. On a billing +/// API that is plausibly a customer's email address, an invoice line, an +/// amount, or an HTML error page from whatever sits in front of Chargebee — and +/// the message reaches the model's context and the turn's durable transcript, +/// where `amount_usd` is already admin-only (#729). +/// +/// So the operator gets the body in the host log and the agent gets the fact. +/// Nothing is lost: an unparseable body is not actionable by a model anyway, +/// which is exactly what makes it the safe one to withhold. Same shape as +/// #688's `PayloadStorage::Refused`. +fn unparsed_body_message(status: u16, body: &str) -> String { + tracing::warn!( + status, + body = %body.chars().take(200).collect::(), + "[chargebee] response body could not be parsed" + ); + format!( + "Chargebee returned {status} with a body this host could not parse. The body is in the \ + host log; it is not reproduced here because its contents are unknown and may carry \ + customer data." + ) +} + +/// Builds the error for a reply whose body could not be used. +fn err_body(status: u16, code: &str, body: &str) -> OpenCompanyError { + OpenCompanyError::Chargebee { + status, + code: code.to_string(), + message: unparsed_body_message(status, body), + } +} + +/// A form body under construction. +/// +/// Deliberately a `Vec` of pairs rather than a map: Chargebee's bracket-array +/// notation repeats a prefix across indices, so key order is meaningful for +/// readability of the encoded body and there are no duplicate keys to collapse. +#[derive(Debug, Default)] +pub struct Form(Vec<(String, String)>); + +impl Form { + /// An empty body. + pub fn new() -> Self { + Self::default() + } + + /// Appends `key=value`. + pub fn push(&mut self, key: impl Into, value: impl Into) { + self.0.push((key.into(), value.into())); + } + + /// Appends `key=value` when `value` is `Some`, and nothing otherwise. + /// + /// Chargebee treats an empty string as an instruction to clear a field, so + /// an omitted optional must be absent from the body rather than blank. + pub fn push_opt(&mut self, key: impl Into, value: Option) { + if let Some(v) = value { + self.0.push((key.into(), v.to_string())); + } + } + + /// Appends `prefix[field][index]=value` — Chargebee's nested-array form. + pub fn push_indexed(&mut self, prefix: &str, field: &str, index: usize, value: impl ToString) { + self.0 + .push((format!("{prefix}[{field}][{index}]"), value.to_string())); + } + + /// The pairs, in insertion order. + pub fn pairs(&self) -> &[(String, String)] { + &self.0 + } +} + +/// A Chargebee API client bound to one site. +#[derive(Clone)] +pub struct ChargebeeClient { + http: reqwest::Client, + api_key: String, + /// Resolved once at construction. Holding the string rather than deriving + /// it per request is what lets a test point the client at a local stub + /// without the production path carrying a test-only branch. + base_url: String, +} + +/// Prints the site and **redacts the API key**. +/// +/// `ChargebeeConfig` already hand-writes this, but the client holds its own copy +/// of the key — so a derived `Debug` here would put a live key into any log line +/// that formatted a client, which is the same leak one level down. Matches +/// `PaypalClient`. +impl std::fmt::Debug for ChargebeeClient { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ChargebeeClient") + .field("base_url", &self.base_url) + .finish_non_exhaustive() + } +} + +impl ChargebeeClient { + /// Builds a client for `config`, talking to that site's real API. + pub fn new(config: ChargebeeConfig) -> Result { + let base_url = config.base_url(); + Self::with_base_url(config, base_url) + } + + /// Builds a client against an explicit base URL, with no trailing slash. + pub fn with_base_url(config: ChargebeeConfig, base_url: String) -> Result { + let http = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(30)) + // Every request carries HTTP Basic with the API key as the + // username. reqwest follows redirects by default and re-sends the + // Authorization header, so a 30x pointing at `http://` would put the + // key on the wire in clear text. Chargebee's API does not redirect, + // so refusing them outright costs nothing and removes the downgrade + // entirely — a scheme check would still leave same-scheme + // redirection to an unintended host. + .redirect(reqwest::redirect::Policy::none()) + .build() + .map_err(|e| OpenCompanyError::Chargebee { + status: 0, + code: "client_build_failed".to_string(), + message: e.to_string(), + })?; + Ok(Self { + http, + api_key: config.api_key, + base_url: base_url.trim_end_matches('/').to_string(), + }) + } + + fn base(&self) -> &str { + &self.base_url + } + + /// `POST path` with a form body, returning the decoded JSON response. + pub async fn post_form( + &self, + path: &str, + form: &Form, + idempotency_key: Option<&str>, + ) -> Result { + self.post_form_replayable(path, form, idempotency_key) + .await + .map(|(body, _)| body) + } + + /// As [`Self::post_form`], and additionally reports whether Chargebee + /// **replayed** a stored response rather than performing the request. + /// + /// Only the invoice path needs this. A replay means no new invoice was + /// raised, which is the correct outcome for a retry and the wrong one for a + /// deliberate second charge — and the two are indistinguishable from the + /// response body, since a replay returns the original invoice verbatim. + pub async fn post_form_replayable( + &self, + path: &str, + form: &Form, + idempotency_key: Option<&str>, + ) -> Result<(Value, bool)> { + let url = format!("{}{}", self.base(), path); + let mut req = self + .http + .post(&url) + .basic_auth(&self.api_key, Some("")) + .form(form.pairs()); + if let Some(key) = idempotency_key { + req = req.header(IDEMPOTENCY_HEADER, key); + } + Self::decode(req.send().await).await + } + + /// `GET path` with query parameters, returning the decoded JSON response. + pub async fn get(&self, path: &str, query: &Form) -> Result { + let url = format!("{}{}", self.base(), path); + let req = self + .http + .get(&url) + .basic_auth(&self.api_key, Some("")) + .query(query.pairs()); + // A GET is never idempotency-replayed; the flag is meaningless here. + Self::decode(req.send().await).await.map(|(body, _)| body) + } + + /// Turns a transport result into either the parsed body or a + /// [`OpenCompanyError::Chargebee`] carrying Chargebee's own error fields. + /// + /// Chargebee reports business failures (`payment_method` not enabled, a + /// customer that does not exist) as a 4xx with a JSON body naming the + /// problem. That body is far more useful to the agent than the status code, + /// so it is preserved rather than flattened into "request failed". + async fn decode( + sent: std::result::Result, + ) -> Result<(Value, bool)> { + let response = sent.map_err(|e| OpenCompanyError::Chargebee { + status: e.status().map(|s| s.as_u16()).unwrap_or(0), + code: "transport_error".to_string(), + // `without_url` keeps the cause and drops the URL reqwest would + // otherwise print. Same reasoning as the body rule below: this text + // reaches the model's context and the durable transcript. + message: e.without_url().to_string(), + })?; + + let status = response.status().as_u16(); + let replayed = response + .headers() + .get(REPLAYED_HEADER) + .and_then(|v| v.to_str().ok()) + .is_some_and(|v| v.eq_ignore_ascii_case("true")); + let body = response + .text() + .await + .map_err(|e| OpenCompanyError::Chargebee { + status, + code: "unreadable_body".to_string(), + message: e.to_string(), + })?; + + let parsed: Value = serde_json::from_str(&body).unwrap_or(Value::Null); + + if (200..300).contains(&status) { + // A success whose body is not a JSON object is not a success we can + // use: `Value::Null` would flow on and every field read would yield + // a default, so a proxy's HTML 200 became an invoice with an empty + // id rather than a reported failure. The raw-body fallback below + // stays for NON-2xx replies, where prose is all there is. + if !parsed.is_object() { + return Err(err_body(status, "unexpected_response", &body)); + } + return Ok((parsed, replayed)); + } + + Err(OpenCompanyError::Chargebee { + status, + code: parsed + .get("api_error_code") + .and_then(Value::as_str) + .unwrap_or("unknown") + .to_string(), + message: parsed + .get("message") + .and_then(Value::as_str) + .map(str::to_string) + // Chargebee's OWN `message` is a classified business outcome + // and belongs in the agent's context. A body without one — a + // proxy's HTML 502, say — is unidentified text and goes to the + // log instead; see `unparsed_body_message`. + .unwrap_or_else(|| unparsed_body_message(status, &body)), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn push_opt_omits_none_rather_than_sending_blank() { + let mut form = Form::new(); + form.push("customer_id", "acme"); + form.push_opt("net_term_days", None::); + form.push_opt("invoice_note", Some("Q3 retainer")); + + let keys: Vec<&str> = form.pairs().iter().map(|(k, _)| k.as_str()).collect(); + assert_eq!(keys, vec!["customer_id", "invoice_note"]); + } + + #[test] + fn indexed_encoding_is_per_field_not_per_object() { + let mut form = Form::new(); + for (i, (desc, amount)) in [("Pro plan", 50_000), ("Setup", 2_500)].iter().enumerate() { + form.push_indexed("charges", "description", i, desc); + form.push_indexed("charges", "amount", i, amount); + } + + let pairs: Vec<(&str, &str)> = form + .pairs() + .iter() + .map(|(k, v)| (k.as_str(), v.as_str())) + .collect(); + assert_eq!( + pairs, + vec![ + ("charges[description][0]", "Pro plan"), + ("charges[amount][0]", "50000"), + ("charges[description][1]", "Setup"), + ("charges[amount][1]", "2500"), + ] + ); + } + + /// A body of exactly the shape that must not be quoted back: an HTML error + /// page from something in front of Chargebee, carrying a customer address + /// and an amount. + const LEAKY_BODY: &str = "Gateway error for alan@tinyhumans.ai — invoice \ + INV-0042, USD 100.00, request 9f3c-aa71"; + + #[test] + fn a_client_debug_does_not_render_its_api_key() { + // The client holds its own copy of the key, so redacting only + // `ChargebeeConfig` leaves the same leak one level down. + let client = ChargebeeClient::new(ChargebeeConfig { + site: "acme-test".to_string(), + api_key: "live_supersecret".to_string(), + }) + .expect("builds"); + let rendered = format!("{client:?}"); + assert!(!rendered.contains("live_supersecret"), "{rendered}"); + assert!(rendered.contains("acme-test"), "{rendered}"); + } + + #[test] + fn an_unparseable_body_is_logged_rather_than_put_in_the_error() { + // This message reaches `ToolResult::error`, so it lands in the model's + // context and the turn's durable transcript. What an unparseable body + // contains is unknown by construction — see `unparsed_body_message`. + let message = unparsed_body_message(502, LEAKY_BODY); + for secret in [ + "alan@tinyhumans.ai", + "INV-0042", + "100.00", + "9f3c-aa71", + "", + ] { + assert!( + !message.contains(secret), + "`{secret}` must not reach the transcript: {message}" + ); + } + // The agent still learns the fact it can act on. + assert!(message.contains("502"), "{message}"); + assert!(message.contains("host log"), "{message}"); + } + + #[test] + fn the_same_rule_applies_to_a_success_whose_body_is_not_an_object() { + let rendered = err_body(200, "unexpected_response", LEAKY_BODY).to_string(); + assert!(!rendered.contains("alan@tinyhumans.ai"), "{rendered}"); + assert!(!rendered.contains("INV-0042"), "{rendered}"); + } + + #[tokio::test] + async fn chargebees_own_error_message_is_still_relayed_verbatim() { + // The narrow half of the rule: a CLASSIFIED Chargebee failure names a + // business outcome the model must read, and withholding it would leave + // the agent unable to tell a refused request from a broken integration. + let app = axum::Router::new().fallback(axum::routing::any(|| async { + ( + axum::http::StatusCode::BAD_REQUEST, + [("content-type", "application/json")], + r#"{"api_error_code":"param_wrong_value","message":"currency_code : INR is not enabled for this site"}"#, + ) + })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("addr"); + let server = tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let client = ChargebeeClient::with_base_url( + ChargebeeConfig { + site: "test".to_string(), + api_key: "cb_key".to_string(), + }, + format!("http://{addr}"), + ) + .expect("client builds"); + let message = client + .get("/invoices/inv_1", &Form::new()) + .await + .expect_err("400 is an error") + .to_string(); + assert!(message.contains("INR is not enabled"), "{message}"); + server.abort(); + } + + #[tokio::test] + async fn a_replayed_post_is_reported_to_the_caller() { + // Chargebee answers a repeated idempotency key with the ORIGINAL + // response, so the body alone cannot distinguish a replay from a fresh + // write. Only this header can. + let app = axum::Router::new().fallback(axum::routing::any(|| async { + ( + axum::http::StatusCode::OK, + [ + ("content-type", "application/json"), + ("chargebee-idempotency-replayed", "true"), + ], + r#"{"invoice":{"id":"inv_1"}}"#, + ) + })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("addr"); + let server = tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let client = ChargebeeClient::with_base_url( + ChargebeeConfig { + site: "test".to_string(), + api_key: "cb_key".to_string(), + }, + format!("http://{addr}"), + ) + .expect("client builds"); + let (_body, replayed) = client + .post_form_replayable("/invoices", &Form::new(), Some("key-1")) + .await + .expect("200"); + assert!(replayed, "the replay header must be surfaced"); + server.abort(); + } +} diff --git a/src/chargebee/mod.rs b/src/chargebee/mod.rs new file mode 100644 index 000000000..f432260d2 --- /dev/null +++ b/src/chargebee/mod.rs @@ -0,0 +1,47 @@ +//! Chargebee billing, integrated as backend service code and surfaced to agents +//! as callable tools (issue #788). +//! +//! The operator's problem this exists for: today they leave OpenCompany, log +//! into Chargebee, create a customer, fill out an invoice form and send it — +//! then repeat the trip to find out whether it was paid. With these tools they +//! say it in chat instead. +//! +//! # Shape +//! +//! - [`types`] — configuration, tool arguments, and the compact projections +//! returned to the agent. Every money field is named `*_in_minor_units`, +//! because Chargebee is minor-unit based and an agent reading "$100" from a +//! prompt will otherwise raise a $1.00 invoice that succeeds. +//! - [`client`] — the REST v2 transport: form-encoded writes, HTTP Basic auth, +//! bracket-array nesting. +//! - [`api`] — the billing operations themselves, validated before any call. +//! +//! The agent-facing bridge lives in [`crate::harness::chargebee`], which turns +//! each `api` function into a tool. The split is deliberate: these shapes are +//! testable without a harness, and a tool description can change without +//! anyone touching the wire format. +//! +//! # Not an MCP server +//! +//! An earlier revision of #788 asked for one, and one was built. The issue was +//! then rewritten to put the integration in the backend service layer instead, +//! which is what this is. The change is not cosmetic — it moves the credential +//! from a separate process's environment into the company's own +//! [`SecretStore`](crate::ports::SecretStore), which is what makes it +//! per-tenant and what lets the console's Billing settings (#527) own it. +//! +//! # Credentials +//! +//! A company's Chargebee API key and site identifier live in its `SecretStore` +//! under [`types::API_KEY_SECRET`] and [`types::SITE_SECRET`], written by the +//! console and never present in a manifest, a tool argument, a tool result, or +//! a log line. No environment variable is consulted: two companies on one host +//! bill two different Chargebee sites, so a process-wide credential could only +//! ever be wrong. + +pub mod api; +pub mod client; +pub mod types; + +pub use client::ChargebeeClient; +pub use types::{API_KEY_SECRET, ChargebeeConfig, SITE_SECRET}; diff --git a/src/chargebee/types.rs b/src/chargebee/types.rs new file mode 100644 index 000000000..ca2016e92 --- /dev/null +++ b/src/chargebee/types.rs @@ -0,0 +1,258 @@ +//! Configuration and argument types for the Chargebee billing tools. +//! +//! Every money-carrying field is named `*_in_minor_units` and typed `i64` on +//! purpose. Chargebee's API is minor-unit based (`amount = 10000` is $100.00 for +//! a two-decimal currency), and an agent filling a field called plain `amount` +//! from the prompt "invoice Alan $100" will write `100` — a $1.00 invoice that +//! succeeds, returns a plausible invoice object, and is wrong by two orders of +//! magnitude. The unit lives in the field name so the mistake has to be made +//! deliberately. +//! +//! This deviates from the tool signatures sketched in issue #788 (`amount: 100`). +//! The deviation is the point: a float dollar amount also invites binary +//! rounding on money, and integer minor units are what Chargebee actually takes. + +use serde::{Deserialize, Serialize}; + +pub use crate::company::billing::{API_KEY_SECRET, SITE_SECRET}; + +/// Connection settings for one company's Chargebee site. +/// +/// `Debug` is hand-written to redact the key — see the impl below. +#[derive(Clone)] +pub struct ChargebeeConfig { + /// The Chargebee site slug, i.e. the `acme` in `acme.chargebee.com`. + pub site: String, + /// The site's API key, sent as the HTTP Basic username. + pub api_key: String, +} + +/// Prints the site and **redacts the key**. +/// +/// Not a nicety. This struct is reachable from `HarnessDeps`, which is a large +/// aggregate that debugging code prints wholesale; a derived `Debug` puts a live +/// Chargebee API key into any log line that ever formats one. Caught by a test +/// that asserted the key could not reach a `Debug` rendering — and, before this +/// impl, it could. +impl std::fmt::Debug for ChargebeeConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ChargebeeConfig") + .field("site", &self.site) + .field("api_key", &"") + .finish() + } +} + +impl ChargebeeConfig { + /// The API v2 base URL for this site, without a trailing slash. + pub fn base_url(&self) -> String { + format!("https://{}.chargebee.com/api/v2", self.site) + } +} + +/// One ad-hoc line item on an invoice. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct ChargeLine { + /// Line-item text shown on the invoice. + pub description: String, + /// The charge amount in the currency's minor unit (cents for USD). + /// Chargebee rejects anything below 1. + pub amount_in_minor_units: i64, +} + +/// Arguments for `chargebee_send_invoice`. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct SendInvoiceArgs { + /// The customer's email. Resolved to a Chargebee customer, and one is + /// created if no match exists — so the agent never has to ask the operator + /// for an internal id it could not know (issue #788, TC-05). + pub customer_email: String, + /// Display name for a customer that has to be created. Ignored when the + /// customer already exists — renaming is not a side effect of invoicing. + #[serde(default)] + pub customer_name: Option, + /// ISO 4217 code, e.g. `USD`. Required: Chargebee will not infer it for a + /// site with more than one enabled currency, and inferring it here would + /// mean guessing what money the operator meant. + pub currency_code: String, + /// At least one line item. + pub line_items: Vec, + /// Days until the invoice falls due. + #[serde(default)] + pub due_days: Option, + /// Free-text note stored on the invoice. + #[serde(default)] + pub invoice_note: Option, + /// Optional `chargebee-idempotency-key`. A retried agent turn that reuses + /// this key gets the original invoice back instead of billing twice. + #[serde(default)] + pub idempotency_key: Option, +} + +/// Arguments for `chargebee_get_invoice`. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct GetInvoiceArgs { + /// The Chargebee invoice id. + pub invoice_id: String, +} + +/// Arguments for `chargebee_list_invoices`. +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +pub struct ListInvoicesArgs { + /// Restrict to one customer by email. Resolved to a customer id first. + #[serde(default)] + pub customer_email: Option, + /// Chargebee invoice status: `paid`, `posted`, `payment_due`, `not_paid`, + /// `voided`, or `pending`. + #[serde(default)] + pub status: Option, + /// Page size, 1-100. Chargebee's own default is 10. + #[serde(default)] + pub limit: Option, +} + +/// Arguments for `chargebee_get_customer`. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct GetCustomerArgs { + /// The email to look up. + pub email: String, +} + +/// Arguments for `chargebee_create_customer`. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct CreateCustomerArgs { + /// Billing email. The identity the other tools resolve against. + pub email: String, + /// Full name, split on the first space into Chargebee's first/last fields. + #[serde(default)] + pub name: Option, + /// Company name, e.g. `Acme Corp`. + #[serde(default)] + pub company: Option, +} + +/// A compact projection of a Chargebee invoice, returned to the agent instead +/// of the raw API object. +/// +/// Chargebee's invoice payload is ~40 fields plus nested line items, billing +/// address and empty collections. Handing that to a model verbatim costs a +/// large amount of context for one invoice and buries the three facts an +/// operator asked about. Anything omitted is still reachable — the agent can +/// call `chargebee_get_invoice` — but the default answer stays legible. +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] +pub struct InvoiceSummary { + /// Chargebee's invoice id. + pub id: String, + /// The customer this invoice belongs to. + pub customer_id: String, + /// `paid`, `payment_due`, `posted`, `voided`, … + pub status: String, + /// ISO 4217 code. + pub currency_code: String, + /// Invoice total, in minor units. + pub total_in_minor_units: i64, + /// Outstanding balance, in minor units. + pub amount_due_in_minor_units: i64, + /// Settled so far, in minor units. + pub amount_paid_in_minor_units: i64, + /// Unix seconds, when Chargebee reports one. + pub due_date: Option, + /// Line-item descriptions, in order. + pub line_items: Vec, + /// A hosted page where the customer can pay, when one could be raised. + /// + /// `None` is not a failure: the payment page is a second API call after the + /// invoice exists, and an invoice that was created but whose link could not + /// be raised is still a real invoice the operator should hear about. + pub payment_url: Option, + /// Set when Chargebee **replayed** an earlier invoice for this idempotency + /// key rather than raising a new one. + /// + /// Serialised only when true, so the ordinary result is unchanged. It has to + /// be reported at all because a replayed response is byte-identical to the + /// original: without this flag a deliberate second identical invoice that + /// was deduped reads exactly like a successful new one, which is a silent + /// failure to bill. + #[serde(skip_serializing_if = "is_false")] + pub replayed_earlier_invoice: bool, +} + +/// `skip_serializing_if` predicate — `bool::not` takes `self` by value and so +/// cannot be used here. +fn is_false(value: &bool) -> bool { + !*value +} + +/// A compact projection of a Chargebee customer. +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] +pub struct CustomerSummary { + /// Chargebee's customer id. + pub id: String, + /// Billing email, when set. + pub email: Option, + /// Display name, assembled from first/last. + pub name: Option, + /// Company name, when set. + pub company: Option, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn base_url_is_the_site_api_v2_root() { + let cfg = ChargebeeConfig { + site: "acme-test".to_string(), + api_key: "cb_test_key".to_string(), + }; + assert_eq!(cfg.base_url(), "https://acme-test.chargebee.com/api/v2"); + } + + #[test] + fn debug_never_renders_the_api_key() { + // Reachable from `HarnessDeps`, which debugging code prints wholesale — + // a derived Debug put a live key in any log line that formatted one. + let rendered = format!( + "{:?}", + ChargebeeConfig { + site: "acme-test".to_string(), + api_key: "cb_live_SUPERSECRET".to_string(), + } + ); + assert!(rendered.contains(""), "{rendered}"); + assert!(!rendered.contains("SUPERSECRET"), "{rendered}"); + assert!( + rendered.contains("acme-test"), + "the site is not secret: {rendered}" + ); + } + + #[test] + fn a_bare_amount_does_not_satisfy_a_line_item() { + // The whole point of the naming convention: "$100" becoming `100` + // must not deserialize into a field that means cents. + assert!( + serde_json::from_str::(r#"{"description":"Consulting","amount":100}"#) + .is_err(), + "a bare `amount` must not satisfy ChargeLine" + ); + let ok: ChargeLine = + serde_json::from_str(r#"{"description":"Consulting","amount_in_minor_units":10000}"#) + .expect("explicit minor units parse"); + assert_eq!(ok.amount_in_minor_units, 10_000); + } + + #[test] + fn send_invoice_needs_only_email_currency_and_lines() { + let args: SendInvoiceArgs = serde_json::from_str( + r#"{"customer_email":"alan@tinyhumans.ai","currency_code":"USD", + "line_items":[{"description":"Consulting","amount_in_minor_units":10000}]}"#, + ) + .expect("minimal args parse"); + assert_eq!(args.customer_email, "alan@tinyhumans.ai"); + assert_eq!(args.due_days, None); + assert_eq!(args.customer_name, None); + assert_eq!(args.line_items.len(), 1); + } +} diff --git a/src/company/billing.rs b/src/company/billing.rs new file mode 100644 index 000000000..673760fce --- /dev/null +++ b/src/company/billing.rs @@ -0,0 +1,25 @@ +//! Secret-store keys for the Chargebee billing integration (issue #788). +//! +//! They live here, always compiled, rather than beside the REST client in +//! `crate::chargebee`: that module is gated on the `chargebee` feature, but the +//! **configuration surface** is not. An operator must be able to open Settings → +//! Billing and see "this build has no Chargebee support" rather than a 404, and +//! the webhook route ships in every build too. Sharing one definition is what +//! keeps the write plane and the read plane from drifting onto two spellings of +//! the same key. + +/// Holds a company's Chargebee API key, written by the console's Billing +/// settings (#527) and read only to authenticate a call. +pub const API_KEY_SECRET: &str = "chargebee/api_key"; + +/// Holds a company's Chargebee site identifier — the `acme-test` in +/// `acme-test.chargebee.com`. +/// +/// Stored beside the key because the pair only makes sense together: a site +/// without its key cannot be called, and a key pointed at the wrong site fails +/// in a way that reads like a bad key. +pub const SITE_SECRET: &str = "chargebee/site"; + +/// Holds the `username:password` pair Chargebee is configured to present on its +/// webhook deliveries, verified by `POST /hooks/{company}/chargebee`. +pub const WEBHOOK_SECRET_KEY: &str = "chargebee/webhook_secret"; diff --git a/src/company/content_test.rs b/src/company/content_test.rs index c87db7178..04d8ba7d9 100644 --- a/src/company/content_test.rs +++ b/src/company/content_test.rs @@ -7,8 +7,9 @@ use std::path::{Path, PathBuf}; use super::{ - CompanyManifest, Tools, grants_composio_explicit, grants_media_explicit, - grants_search_explicit, load_dir_skills, parse_workflow, walk_workspace, + CompanyManifest, Tools, grants_chargebee_explicit, grants_composio_explicit, + grants_media_explicit, grants_paypal_explicit, grants_search_explicit, load_dir_skills, + parse_workflow, walk_workspace, }; use crate::runtime::builder::effective_grants; @@ -383,6 +384,33 @@ fn granting_search_never_strips_the_inherited_default_belt() { } } +#[test] +fn a_wildcard_never_confers_a_billing_namespace() { + // The point of these helpers: `*` is set for file and shell tools and must + // not quietly hand out invoicing or a wallet balance. + for grants in [ + vec!["*".to_string()], + vec!["workspace".to_string(), "*".to_string()], + vec![], + vec!["chargebeeish".to_string(), "paypalish".to_string()], + vec!["mcp:chargebee".to_string()], + ] { + assert!(!grants_chargebee_explicit(&grants), "{grants:?}"); + assert!(!grants_paypal_explicit(&grants), "{grants:?}"); + } +} + +#[test] +fn a_billing_namespace_is_granted_bare_or_dotted_and_never_by_its_sibling() { + assert!(grants_chargebee_explicit(&["chargebee".to_string()])); + assert!(grants_chargebee_explicit(&["chargebee.read".to_string()])); + assert!(grants_paypal_explicit(&["paypal".to_string()])); + assert!(grants_paypal_explicit(&["paypal.wallet".to_string()])); + // Two namespaces, neither implying the other. + assert!(!grants_paypal_explicit(&["chargebee".to_string()])); + assert!(!grants_chargebee_explicit(&["paypal".to_string()])); +} + #[test] fn the_repo_skill_registry_parses() { let skills = load_dir_skills(&repo_root().join("skills")) diff --git a/src/company/mod.rs b/src/company/mod.rs index 82d2e703b..380945338 100644 --- a/src/company/mod.rs +++ b/src/company/mod.rs @@ -32,11 +32,13 @@ pub mod copilot; // How this instance obtains its TinyHumans credential (projected, rotating // platform token vs a static key). Always compiled: the answer decides whether a // company can think at all, in every build. +pub mod billing; pub mod credentials; pub mod dns; pub mod inference; mod manifest; pub mod mcp; +pub mod paypal; // Console MCP OAuth (issue #90): discovery + PKCE + DCR + token exchange for the // per-tenant browser sign-in flow. Needs the vendored `oh::mcp::config_servers` discovery // primitive + `uuid`/`base64`/`url`, so it links only under the `mcp` feature. @@ -117,7 +119,8 @@ pub use types::{ INFERENCE_TIERS, Inference, KNOWN_CHANNELS, MAX_DELEGATION_DEPTH_BOUNDS, McpServer, ORCHESTRATOR_TIER, PLAN_NAMES, PLAN_PERIODS, POLICY_MODES, PROMPT_CLASSES, PROMPT_FILE_BUDGET_CHARS, PROVISIONED_POLICY_MODE, Place, Plan, Policy, Schedule, Skill, TIERS, - TOOL_PROVIDERS, Tools, grants_composio_explicit, grants_media_explicit, grants_repo_explicit, + TOOL_PROVIDERS, Tools, grants_chargebee_explicit, grants_composio_explicit, + grants_media_explicit, grants_paypal_explicit, grants_repo_explicit, grants_repo_write_explicit, grants_search_explicit, grants_workspace_write_explicit, orchestrator_id, }; diff --git a/src/company/paypal.rs b/src/company/paypal.rs new file mode 100644 index 000000000..81442b81f --- /dev/null +++ b/src/company/paypal.rs @@ -0,0 +1,112 @@ +//! Secret-store keys and environment selection for the PayPal integration +//! (issue #789). +//! +//! Always compiled, for the same reason as [`crate::company::billing`]: the REST +//! client is gated on the `paypal` feature but the configuration surface is not, +//! so an operator sees "this build has no PayPal support" rather than a 404. + +use serde::{Deserialize, Serialize}; + +/// Holds a company's PayPal REST app client id. +pub const CLIENT_ID_SECRET: &str = "paypal/client_id"; + +/// Holds a company's PayPal REST app secret. +pub const CLIENT_SECRET_SECRET: &str = "paypal/client_secret"; + +/// Holds `sandbox` or `live` — which PayPal environment the credentials belong +/// to. +/// +/// Stored rather than inferred because the two are indistinguishable from the +/// credential itself: a sandbox client id against the live host authenticates +/// as nobody, and the error says "invalid client", which reads as a typo rather +/// than as pointing at the wrong world. +pub const ENVIRONMENT_SECRET: &str = "paypal/environment"; + +/// Which PayPal environment a company's credentials belong to. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum PaypalEnvironment { + /// developer.paypal.com test accounts. The default: a mis-set environment + /// should read fake money, never move real money. + #[default] + Sandbox, + /// Real accounts, real balances. + Live, +} + +impl PaypalEnvironment { + /// The API base for this environment, without a trailing slash. + pub fn base_url(self) -> &'static str { + match self { + Self::Sandbox => "https://api-m.sandbox.paypal.com", + Self::Live => "https://api-m.paypal.com", + } + } + + /// Parses a stored/`PUT` value, defaulting to sandbox. + /// + /// Anything unrecognised is sandbox, not an error: the failure mode of + /// guessing wrong here is reading a fake balance, whereas defaulting to + /// `live` on a typo would point an agent at real money. + pub fn parse(raw: &str) -> Self { + match raw.trim().to_ascii_lowercase().as_str() { + "live" | "production" => Self::Live, + _ => Self::Sandbox, + } + } + + /// The stored spelling. + pub fn as_str(self) -> &'static str { + match self { + Self::Sandbox => "sandbox", + Self::Live => "live", + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn an_unrecognised_environment_falls_back_to_sandbox_not_live() { + // The whole point: a typo must not aim an agent at real money. + for raw in [ + "", " ", "sandbox", "SANDBOX", "prod", "liv", "nonsense", "Live-ish", + ] { + assert_eq!( + PaypalEnvironment::parse(raw), + PaypalEnvironment::Sandbox, + "{raw:?} must not resolve to live" + ); + } + // Only the two exact spellings reach live. + assert_eq!(PaypalEnvironment::parse("live"), PaypalEnvironment::Live); + assert_eq!(PaypalEnvironment::parse(" LIVE "), PaypalEnvironment::Live); + assert_eq!( + PaypalEnvironment::parse("production"), + PaypalEnvironment::Live + ); + } + + #[test] + fn each_environment_names_its_own_host() { + assert_eq!( + PaypalEnvironment::Sandbox.base_url(), + "https://api-m.sandbox.paypal.com" + ); + assert_eq!( + PaypalEnvironment::Live.base_url(), + "https://api-m.paypal.com" + ); + assert_ne!( + PaypalEnvironment::Sandbox.base_url(), + PaypalEnvironment::Live.base_url() + ); + } + + #[test] + fn the_default_is_sandbox() { + assert_eq!(PaypalEnvironment::default(), PaypalEnvironment::Sandbox); + } +} diff --git a/src/company/runtime.rs b/src/company/runtime.rs index 677daa09e..e6b37f7aa 100644 --- a/src/company/runtime.rs +++ b/src/company/runtime.rs @@ -2336,6 +2336,10 @@ mod tests { plan, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, search: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), diff --git a/src/company/types.rs b/src/company/types.rs index 3d934665d..d2950983a 100644 --- a/src/company/types.rs +++ b/src/company/types.rs @@ -159,6 +159,32 @@ pub fn grants_composio_explicit(grants: &[String]) -> bool { .any(|grant| grant == "composio" || grant.starts_with("composio.")) } +/// Whether a tool-grant list **explicitly** grants the `chargebee` billing +/// namespace (issue #788). +/// +/// Like [`grants_composio_explicit`], the catch-all `*` does **not** grant it: +/// these tools send invoices to real customers of a real business, so they are +/// opted into by name rather than ridden in on a wildcard a company set for its +/// file and shell tools. Lives here (always compiled) so the feature-gated +/// harness wiring and the always-compiled console capability route key off one +/// source of truth. +pub fn grants_chargebee_explicit(grants: &[String]) -> bool { + grants + .iter() + .any(|grant| grant == "chargebee" || grant.starts_with("chargebee.")) +} + +/// Whether a tool-grant list **explicitly** grants the `paypal` namespace +/// (issue #789). +/// +/// Like its siblings, the catch-all `*` does **not** grant it. These tools read +/// a real business's wallet, so they are opted into by name. +pub fn grants_paypal_explicit(grants: &[String]) -> bool { + grants + .iter() + .any(|grant| grant == "paypal" || grant.starts_with("paypal.")) +} + /// Whether a tool-grant list **explicitly** grants the metered `search` /// namespace (issue #238). /// diff --git a/src/error.rs b/src/error.rs index 546337022..32f0b5271 100644 --- a/src/error.rs +++ b/src/error.rs @@ -216,6 +216,36 @@ pub enum OpenCompanyError { message: String, }, + /// A Chargebee Billing API failure, or a tool argument this crate rejected + /// before making the call (issue #788). + /// + /// Carries `status` alongside `code` because Chargebee reports business + /// outcomes — a customer that does not exist, a currency the site has not + /// enabled — as 4xx responses whose JSON body names the real problem. The + /// agent needs that body, not the status, so both are preserved; a locally + /// rejected argument uses `status: 0` and `code: invalid_arguments`. + #[error("chargebee error ({code}): {message}")] + Chargebee { + /// The HTTP status, or `0` when the failure never reached the network. + status: u16, + /// Chargebee's `api_error_code`, or a local token. + code: String, + /// A human-readable description of the failure. + message: String, + }, + + /// A PayPal REST API failure, or an argument rejected before the call + /// (issue #789). + #[error("paypal error ({code}): {message}")] + Paypal { + /// The HTTP status, or `0` when the failure never reached the network. + status: u16, + /// PayPal's `name`/`error` token, or a local one. + code: String, + /// A human-readable description of the failure. + message: String, + }, + /// A spawned background task the caller was waiting on panicked or was /// aborted, so its result never arrived (issue #383). /// @@ -296,6 +326,8 @@ impl OpenCompanyError { Self::Orchestration { code, .. } => code.clone(), Self::Tinyplace { code, .. } => format!("tinyplace_{code}"), Self::TinyHumans { code, .. } => format!("tinyhumans_{code}"), + Self::Chargebee { code, .. } => format!("chargebee_{code}"), + Self::Paypal { code, .. } => format!("paypal_{code}"), Self::BackgroundTask(_) => "background_task".to_string(), Self::Unimplemented(_) => "unimplemented".to_string(), #[cfg(feature = "openhuman")] diff --git a/src/harness/brain.rs b/src/harness/brain.rs index b66515059..5e74445ea 100644 --- a/src/harness/brain.rs +++ b/src/harness/brain.rs @@ -2938,6 +2938,10 @@ description = "Runs Acme." plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -3105,6 +3109,10 @@ description = "Builds it." plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -3221,6 +3229,10 @@ members = ["engineer"] plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -5125,6 +5137,10 @@ members = ["engineer"] plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -6077,6 +6093,10 @@ members = ["eng1", "eng2"] plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -6217,6 +6237,10 @@ members = ["eng1", "eng2"] plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -6303,6 +6327,10 @@ members = ["eng1", "eng2"] plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -6629,6 +6657,10 @@ members = ["eng1", "eng2"] plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -7126,6 +7158,10 @@ members = ["eng1", "eng2"] plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -7444,6 +7480,10 @@ members = ["eng1", "eng2"] plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer, run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -7840,6 +7880,10 @@ members = ["eng1", "eng2"] plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer, run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, diff --git a/src/harness/build.rs b/src/harness/build.rs index 89e300259..52b6344ae 100644 --- a/src/harness/build.rs +++ b/src/harness/build.rs @@ -440,6 +440,51 @@ pub fn build_agent( } } + // Issue #788: Chargebee billing. Same fail-closed shape as `composio` + // above, and for a sharper reason — these tools send invoices to a real + // business's real customers. Two conditions, both required: + // + // 1. an **EXPLICIT** `chargebee` grant. The catch-all `*` does NOT confer + // it, following the media/composio/search precedent. + // 2. a resolved per-company connection on the deps (`deps.chargebee`), + // read from THAT company's secret store by the runtime builder. + // + // A grant with no credential wires nothing and warns: an agent told it can + // bill, that silently cannot, is better than one billing through somebody + // else's Chargebee site. + #[cfg(feature = "chargebee")] + if crate::company::grants_chargebee_explicit(grants) { + match &deps.chargebee { + Some(config) => { + tools.extend(crate::harness::chargebee::chargebee_tools(config)); + } + None => tracing::warn!( + company = %company, + agent = %manifest_agent.id, + "[build] agent explicitly grants `chargebee` but no per-company Chargebee \ + credentials are configured; billing tools NOT wired (fail-closed)" + ), + } + } + + // Issue #789: PayPal wallet reads. Same fail-closed shape as `chargebee` + // above — an explicit `paypal` grant AND a resolved per-company credential. + // Both tools are read-only, so nothing here can move money; the grant is + // still opt-in by name because a wallet balance is a business's private + // figure, not something a `*` wildcard should hand out. + #[cfg(feature = "paypal")] + if crate::company::grants_paypal_explicit(grants) { + match &deps.paypal { + Some(config) => tools.extend(crate::harness::paypal::paypal_tools(config)), + None => tracing::warn!( + company = %company, + agent = %manifest_agent.id, + "[build] agent explicitly grants `paypal` but no per-company PayPal credentials \ + are configured; wallet tools NOT wired (fail-closed)" + ), + } + } + // Metered web search (issue #238) — the discovery tool the `web` namespace // never had. `web_fetch` / `http_request` / `curl` read a URL the agent // already has; nothing could find one, while three shipped skills instruct @@ -1485,6 +1530,10 @@ mod tests { plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, diff --git a/src/harness/chargebee.rs b/src/harness/chargebee.rs new file mode 100644 index 000000000..71781a841 --- /dev/null +++ b/src/harness/chargebee.rs @@ -0,0 +1,572 @@ +//! The agent-facing bridge for Chargebee billing (issue #788): five tools over +//! the operations in [`crate::chargebee::api`]. +//! +//! # Why the credential is per company and resolved late +//! +//! Two companies on one host bill two different Chargebee sites, so a +//! process-wide key could only ever be wrong for one of them. The API key and +//! site identifier are read from **that company's** [`SecretStore`], written by +//! the console's Billing settings (#527). No environment variable is consulted. +//! +//! Resolution happens at roster-build time and is folded into the harness +//! fingerprint, so an operator who sets or rotates the key in the console gets +//! it on the next turn rather than after a restart — the same contract Composio +//! has. +//! +//! # Fail closed +//! +//! Tools are wired only when a company **explicitly** grants `chargebee` *and* +//! both secrets resolve. A catch-all `*` does not confer it: these tools move +//! money and send invoices to real people, so they are opted into by name rather +//! than ridden in on a wildcard set for file and shell tools. A grant with no +//! credential wires nothing and warns — never a borrowed identity. +//! +//! # Approval +//! +//! `chargebee_send_invoice` and `chargebee_create_customer` write to a billing +//! system a real customer sees, so they are [`PermissionLevel::Execute`] and +//! park through the harness approval policy. The three read tools are +//! [`PermissionLevel::ReadOnly`] and never park — asking "has Alan paid?" +//! should not need a click. + +use std::sync::Arc; + +use crate::chargebee::types::{API_KEY_SECRET, ChargebeeConfig, SITE_SECRET}; +use crate::ports::SecretStore; +use crate::ports::types::CompanyId; + +/// One company's resolved Chargebee connection. +#[derive(Clone, Debug)] +pub struct TenantChargebee { + config: ChargebeeConfig, +} + +impl TenantChargebee { + /// Resolves a company's Chargebee credentials from its secret store. + /// + /// `Ok(None)` when either half is missing. Both are required and the pair is + /// meaningless apart: a site with no key cannot be called, and a key pointed + /// at the wrong site fails in a way that reads like a bad key. + /// + /// A store **read failure** is an `Err`, not `Ok(None)`. Collapsing the two + /// would make an unhealthy secret store indistinguishable from "no + /// credential configured", and the caller's response to those differs + /// completely: absence should wire no tools, while a transient read error + /// should keep the connection it already had. Deciding that here, rather + /// than at the caller, is what makes the choice visible — see + /// `HarnessPool::resolve_chargebee`. + pub async fn resolve( + secrets: &Arc, + company: &CompanyId, + ) -> crate::error::Result> { + let read = async |key: &str| -> crate::error::Result> { + Ok(secrets + .get(company, key) + .await? + .map(|value| value.0.trim().to_string()) + .filter(|value| !value.is_empty())) + }; + let (Some(site), Some(api_key)) = (read(SITE_SECRET).await?, read(API_KEY_SECRET).await?) + else { + return Ok(None); + }; + Ok(Some(TenantChargebee { + config: ChargebeeConfig { site, api_key }, + })) + } + + /// The Chargebee site this company bills through. Never the key. + pub fn site(&self) -> &str { + &self.config.site + } + + /// A stable hash of the connection, for the roster staleness check. + /// + /// Covers the site AND the key, so rotating a key with the site unchanged + /// still rebuilds the roster — otherwise a rotated credential would keep + /// authenticating with the old one until a restart. + pub fn fingerprint(config: &Option) -> u64 { + use std::hash::{Hash, Hasher}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + match config { + None => 0u8.hash(&mut hasher), + Some(c) => { + 1u8.hash(&mut hasher); + c.config.site.hash(&mut hasher); + c.config.api_key.hash(&mut hasher); + } + } + hasher.finish() + } +} + +#[cfg(feature = "chargebee")] +pub use live::chargebee_tools; + +#[cfg(feature = "chargebee")] +mod live { + use super::*; + + use anyhow::Result; + use async_trait::async_trait; + use serde_json::{Value, json}; + + use crate::chargebee::api; + use crate::chargebee::client::ChargebeeClient; + use crate::chargebee::types::{ + CreateCustomerArgs, GetCustomerArgs, GetInvoiceArgs, ListInvoicesArgs, SendInvoiceArgs, + }; + + use oh::tools::traits::{PermissionLevel, Tool, ToolResult}; + use openhuman_core::openhuman as oh; + + /// Builds the five per-tenant Chargebee tools over a resolved connection. + pub fn chargebee_tools(config: &TenantChargebee) -> Vec> { + let config = Arc::new(config.clone()); + vec![ + Box::new(SendInvoiceTool(Arc::clone(&config))), + Box::new(GetInvoiceTool(Arc::clone(&config))), + Box::new(ListInvoicesTool(Arc::clone(&config))), + Box::new(GetCustomerTool(Arc::clone(&config))), + Box::new(CreateCustomerTool(config)), + ] + } + + /// Builds the HTTP client for a call about to be made. + /// + /// Per call rather than once at construction so a key rotated mid-roster is + /// never held open by a long-lived connection built from the old one. + fn client(config: &TenantChargebee) -> crate::error::Result { + ChargebeeClient::new(config.config.clone()) + } + + /// Renders a successful tool result, or the failure as text the agent can + /// act on. + /// + /// A Chargebee rejection ("that currency is not enabled on this site") is a + /// [`ToolResult::error`], not a transport failure: the call dispatched fine + /// and produced an answer worth relaying. Collapsing the two would leave the + /// agent unable to tell a broken integration from a business outcome. + fn render(what: &str, outcome: crate::error::Result) -> ToolResult { + match outcome { + Ok(value) => match serde_json::to_string_pretty(&value) { + Ok(text) => ToolResult::success(text), + Err(e) => { + ToolResult::error(format!("{what} succeeded but could not be rendered: {e}")) + } + }, + Err(e) => ToolResult::error(format!("{what} failed: {e}")), + } + } + + /// Parses tool arguments, reporting a bad shape as a tool error rather than + /// failing the turn. + fn parse(args: Value) -> std::result::Result { + serde_json::from_value(args) + .map_err(|e| ToolResult::error(format!("invalid arguments: {e}"))) + } + + /// The minor-unit warning, repeated in every schema that takes money. An + /// agent reads one tool's schema, not the module docs. + const MINOR_UNITS: &str = "Amount in the currency's MINOR unit. $100.00 USD is 10000, not 100."; + + pub struct SendInvoiceTool(Arc); + + #[async_trait] + impl Tool for SendInvoiceTool { + fn name(&self) -> &str { + "chargebee_send_invoice" + } + + fn description(&self) -> &str { + "Create and send a Chargebee invoice to a customer, identified by email. Creates the \ + customer automatically if no Chargebee record matches that email, so you never need \ + an internal customer id. Raises an UNPAID invoice — it does not charge a stored card \ + — and returns the invoice with a payment link when one can be raised." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "required": ["customer_email", "currency_code", "line_items"], + "additionalProperties": false, + "properties": { + "customer_email": {"type": "string", "description": "Who to invoice."}, + "customer_name": { + "type": "string", + "description": "Only used if the customer has to be created; an existing customer is never renamed." + }, + "currency_code": {"type": "string", "description": "ISO 4217, e.g. USD. Must be enabled on the Chargebee site."}, + "line_items": { + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "required": ["description", "amount_in_minor_units"], + "additionalProperties": false, + "properties": { + "description": {"type": "string"}, + "amount_in_minor_units": {"type": "integer", "minimum": 1, "description": MINOR_UNITS} + } + } + }, + "due_days": {"type": "integer", "minimum": 0, "description": "Days until the invoice falls due."}, + "invoice_note": {"type": "string"}, + "idempotency_key": { + "type": "string", + "description": "Rarely needed. One is derived from the invoice automatically, so a retry of the same invoice cannot bill the customer twice. Supply a distinct value ONLY to raise a second, deliberately identical invoice for the same customer." + } + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + // A real customer receives this. It parks. + PermissionLevel::Execute + } + + async fn execute(&self, args: Value) -> Result { + let args: SendInvoiceArgs = match parse(args) { + Ok(args) => args, + Err(result) => return Ok(result), + }; + let client = match client(&self.0) { + Ok(client) => client, + Err(e) => return Ok(ToolResult::error(format!("chargebee client: {e}"))), + }; + // Deliberately no customer email: this line lands in durable host + // logs, and a counterparty's address is their personal data, not + // ours to retain for operational telemetry. The site and line count + // are enough to trace a call. + tracing::info!( + site = %self.0.site(), + line_items = args.line_items.len(), + "[chargebee] send_invoice" + ); + Ok(render( + "chargebee_send_invoice", + api::send_invoice(&client, args).await, + )) + } + } + + pub struct GetInvoiceTool(Arc); + + #[async_trait] + impl Tool for GetInvoiceTool { + fn name(&self) -> &str { + "chargebee_get_invoice" + } + + fn description(&self) -> &str { + "Fetch one Chargebee invoice by id, with its current status (paid, payment_due, \ + voided), amount due and amount paid. Use this to answer whether an invoice has been \ + paid." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "required": ["invoice_id"], + "additionalProperties": false, + "properties": {"invoice_id": {"type": "string"}} + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + async fn execute(&self, args: Value) -> Result { + let args: GetInvoiceArgs = match parse(args) { + Ok(args) => args, + Err(result) => return Ok(result), + }; + let client = match client(&self.0) { + Ok(client) => client, + Err(e) => return Ok(ToolResult::error(format!("chargebee client: {e}"))), + }; + Ok(render( + "chargebee_get_invoice", + api::get_invoice(&client, args).await, + )) + } + } + + pub struct ListInvoicesTool(Arc); + + #[async_trait] + impl Tool for ListInvoicesTool { + fn name(&self) -> &str { + "chargebee_list_invoices" + } + + fn description(&self) -> &str { + "List Chargebee invoices, optionally narrowed to one customer by email and/or a \ + status. An email that matches no Chargebee customer returns an empty list, never the \ + whole site's invoices." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "additionalProperties": false, + "properties": { + "customer_email": {"type": "string"}, + "status": { + "type": "string", + "enum": ["paid", "posted", "payment_due", "not_paid", "voided", "pending"] + }, + "limit": {"type": "integer", "minimum": 1, "maximum": 100} + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + async fn execute(&self, args: Value) -> Result { + let args: ListInvoicesArgs = match parse(args) { + Ok(args) => args, + Err(result) => return Ok(result), + }; + let client = match client(&self.0) { + Ok(client) => client, + Err(e) => return Ok(ToolResult::error(format!("chargebee client: {e}"))), + }; + Ok(render( + "chargebee_list_invoices", + api::list_invoices(&client, args).await, + )) + } + } + + pub struct GetCustomerTool(Arc); + + #[async_trait] + impl Tool for GetCustomerTool { + fn name(&self) -> &str { + "chargebee_get_customer" + } + + fn description(&self) -> &str { + "Look up a Chargebee customer by email. Returns nothing when no customer matches — \ + which is not an error. `chargebee_send_invoice` already creates a missing customer, \ + so you rarely need this first." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "required": ["email"], + "additionalProperties": false, + "properties": {"email": {"type": "string"}} + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + async fn execute(&self, args: Value) -> Result { + let args: GetCustomerArgs = match parse(args) { + Ok(args) => args, + Err(result) => return Ok(result), + }; + let client = match client(&self.0) { + Ok(client) => client, + Err(e) => return Ok(ToolResult::error(format!("chargebee client: {e}"))), + }; + match api::get_customer(&client, &args.email).await { + // "No such customer" is an answer, not a failure — an error here + // would push the agent into apologising for a successful lookup. + Ok(None) => Ok(ToolResult::success(format!( + "No Chargebee customer matches {}.", + args.email + ))), + other => Ok(render("chargebee_get_customer", other)), + } + } + } + + pub struct CreateCustomerTool(Arc); + + #[async_trait] + impl Tool for CreateCustomerTool { + fn name(&self) -> &str { + "chargebee_create_customer" + } + + fn description(&self) -> &str { + "Create a Chargebee customer. Only needed to record someone ahead of invoicing them — \ + `chargebee_send_invoice` creates a missing customer on its own." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "required": ["email"], + "additionalProperties": false, + "properties": { + "email": {"type": "string"}, + "name": {"type": "string", "description": "Full name; split into first/last."}, + "company": {"type": "string"} + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::Execute + } + + async fn execute(&self, args: Value) -> Result { + let args: CreateCustomerArgs = match parse(args) { + Ok(args) => args, + Err(result) => return Ok(result), + }; + let client = match client(&self.0) { + Ok(client) => client, + Err(e) => return Ok(ToolResult::error(format!("chargebee client: {e}"))), + }; + Ok(render( + "chargebee_create_customer", + api::create_customer(&client, args).await, + )) + } + } +} + +#[cfg(all(test, feature = "chargebee"))] +mod tests { + use super::*; + use crate::ports::types::SecretValue; + use crate::store::fs::FsSecretStore; + + async fn store(entries: &[(&str, &str)]) -> (Arc, CompanyId) { + let dir = tempfile::tempdir().expect("tempdir"); + let store: Arc = Arc::new(FsSecretStore::new(dir.keep())); + let company = CompanyId::new("acme"); + for (key, value) in entries { + store + .set(&company, key, SecretValue(value.to_string())) + .await + .expect("set"); + } + (store, company) + } + + #[tokio::test] + async fn both_halves_are_required() { + // Neither half alone is usable, and half-configured must fail closed + // rather than call the wrong site or send no auth. + let (only_site, company) = store(&[(SITE_SECRET, "acme-test")]).await; + assert!( + TenantChargebee::resolve(&only_site, &company) + .await + .expect("a readable store is not an error") + .is_none() + ); + + let (only_key, company) = store(&[(API_KEY_SECRET, "cb_key")]).await; + assert!( + TenantChargebee::resolve(&only_key, &company) + .await + .expect("a readable store is not an error") + .is_none() + ); + + let (neither, company) = store(&[]).await; + assert!( + TenantChargebee::resolve(&neither, &company) + .await + .expect("a readable store is not an error") + .is_none() + ); + } + + #[test] + fn the_fingerprint_moves_on_either_half_and_is_stable_otherwise() { + // This function is the whole input to the roster staleness check, so a + // half of the pair dropped out of the hash would silently stop + // rebuilding: agents would keep authenticating with a revoked key until + // the process restarted, with nothing failing to say so. + let of = |site: &str, key: &str| { + TenantChargebee::fingerprint(&Some(TenantChargebee { + config: ChargebeeConfig { + site: site.to_string(), + api_key: key.to_string(), + }, + })) + }; + + let base = of("acme-test", "cb_key"); + assert_eq!(base, of("acme-test", "cb_key"), "stable for one config"); + assert_ne!(base, of("acme-live", "cb_key"), "the site must count"); + assert_ne!(base, of("acme-test", "cb_rotated"), "the KEY must count"); + assert_ne!( + base, + TenantChargebee::fingerprint(&None), + "connected and unconnected must differ" + ); + } + + #[tokio::test] + async fn a_blank_secret_counts_as_absent() { + // The console writing an empty string is a cleared field, not a + // credential — resolving it would produce requests with no auth. + let (store, company) = store(&[(SITE_SECRET, "acme-test"), (API_KEY_SECRET, " ")]).await; + assert!( + TenantChargebee::resolve(&store, &company) + .await + .expect("a readable store is not an error") + .is_none() + ); + } + + #[tokio::test] + async fn a_complete_pair_resolves_and_never_exposes_the_key() { + let (store, company) = + store(&[(SITE_SECRET, " acme-test "), (API_KEY_SECRET, " cb_key ")]).await; + let resolved = TenantChargebee::resolve(&store, &company) + .await + .expect("the store reads") + .expect("both halves present"); + assert_eq!(resolved.site(), "acme-test", "whitespace is trimmed"); + // `site()` is the only accessor; there is deliberately no key getter, + // and Debug must not become one by accident. + assert!( + !format!("{resolved:?}").contains("cb_key"), + "the API key must not reach a Debug rendering" + ); + } + + #[test] + fn the_five_tools_split_reads_from_writes() { + use oh::tools::traits::PermissionLevel; + use openhuman_core::openhuman as oh; + + let config = TenantChargebee { + config: ChargebeeConfig { + site: "acme-test".to_string(), + api_key: "cb_key".to_string(), + }, + }; + let tools = live::chargebee_tools(&config); + let by_name: Vec<(&str, PermissionLevel)> = tools + .iter() + .map(|t| (t.name(), t.permission_level())) + .collect(); + assert_eq!(by_name.len(), 5); + + for (name, level) in by_name { + let expected = match name { + // Writes a real customer sees. Parks for approval. + "chargebee_send_invoice" | "chargebee_create_customer" => PermissionLevel::Execute, + // "Has Alan paid?" must not need a click. + _ => PermissionLevel::ReadOnly, + }; + assert_eq!(level, expected, "{name} permission level"); + } + } +} diff --git a/src/harness/composio_turn_test.rs b/src/harness/composio_turn_test.rs index 610d2eb85..95d26ebac 100644 --- a/src/harness/composio_turn_test.rs +++ b/src/harness/composio_turn_test.rs @@ -360,6 +360,10 @@ async fn harness( // An empty toolkit allowlist is "defer to the backend" (open mode) — // the worst case for catalogue size, and the case a newly-connected // provider lands in. + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, composio: Some(TenantComposio::new( composio_url, Credential::from_value("stub-tenant-token"), diff --git a/src/harness/mod.rs b/src/harness/mod.rs index 3f46c091e..d2dc7bd25 100644 --- a/src/harness/mod.rs +++ b/src/harness/mod.rs @@ -51,6 +51,8 @@ pub mod audit; pub mod brain; pub mod build; pub mod capability_budget; +#[cfg(feature = "chargebee")] +pub mod chargebee; pub mod composio; /// Issue #410: how a Composio action catalogue is narrowed and rendered for an /// agent, and why every cut it makes describes itself. Pure and un-gated (the @@ -79,6 +81,14 @@ pub mod mcp_probe; pub mod memory; pub mod memory_loop; pub mod orchestrator; +/// Chargebee billing tools (issue #788), wired per company from its own +/// SecretStore. Always compiled so the credential resolution and the fail-closed +/// decision are testable at default features; only the tools are gated. +/// PayPal wallet + transaction tools (issue #789), wired per company from its +/// own SecretStore. Always compiled so credential resolution and the +/// fail-closed decision are testable at default features. +#[cfg(feature = "paypal")] +pub mod paypal; /// Issue #337: the planning station — one tool-less model call per card entering /// `planning`, with the host gathering the evidence and verifying every /// prerequisite the model claims. See [`planning`]. @@ -416,6 +426,21 @@ pub struct HarnessDeps { /// else this instance's platform identity. With neither, no tools are wired — /// never a borrowed identity. pub composio: Option, + + /// The per-company Chargebee connection (issue #788). `None` (the default at + /// every construction site) fails closed — no billing tools are wired. + /// Resolved from that company's own secret store, never from the + /// environment: two companies on one host bill two different sites. + /// `HarnessPool::ensure` re-resolves it each turn, so a key set or rotated in + /// the console takes effect next turn with no restart. + #[cfg(feature = "chargebee")] + pub chargebee: Option, + + /// The per-company PayPal connection (issue #789). `None` fails closed — + /// no wallet tools are wired. Resolved from that company's own secret store + /// and re-resolved each turn, like `chargebee`. + #[cfg(feature = "paypal")] + pub paypal: Option, /// The MANAGED web-search backend (issue #238). `None` (the default at every /// construction site but the production runtime builder) **fails closed** — /// no `web_search` tool is wired and agents behave exactly as before. @@ -863,6 +888,14 @@ pub struct HarnessPool { /// store wired the config is the static [`HarnessDeps::composio`], whose /// fingerprint never moves. composio_fingerprints: RwLock>, + /// Fingerprint of the billing connections (Chargebee #788, PayPal #789) the + /// cached roster was built from, keyed by company. + /// + /// Without this axis a credential saved from the console reaches nothing + /// until a restart — the roster is cached, so `build_agent` is never called + /// again to notice it. That was live for both integrations until the tools + /// were observed missing from an agent whose settings page said "Connected". + billing_fingerprints: RwLock>, /// Fingerprint of the company's bound-repository set the cached roster was /// built from, keyed by company (issue #245). Drives repository freshness: /// [`ensure`](Self::ensure) re-reads the binding index from the @@ -977,6 +1010,7 @@ impl HarnessPool { overlay_fingerprints: RwLock::new(HashMap::new()), capability_fingerprints: RwLock::new(HashMap::new()), composio_fingerprints: RwLock::new(HashMap::new()), + billing_fingerprints: RwLock::new(HashMap::new()), repo_fingerprints: RwLock::new(HashMap::new()), skill_fingerprints: RwLock::new(HashMap::new()), budget_fingerprints: RwLock::new(HashMap::new()), @@ -1092,6 +1126,29 @@ impl HarnessPool { let composio_config = self.resolve_composio(company, deps).await; let composio_fp = composio::TenantComposio::fingerprint(&composio_config); + // Re-resolve + fingerprint the billing connections (#788, #789) for the + // same reason as Composio above: both are set from the console, so a + // roster that never re-reads them leaves an agent without billing tools + // on a company whose settings page reads "Connected". + #[cfg(feature = "chargebee")] + let chargebee_config = self.resolve_chargebee(company, deps).await; + #[cfg(feature = "paypal")] + let paypal_config = self.resolve_paypal(company, deps).await; + // A build without either feature has no billing axis to go stale on, so + // the fingerprint is a constant and this company never rebuilds on it. + let billing_fp = { + use std::hash::Hasher; + // `mut` is only exercised when a billing feature is compiled in; a + // build with neither writes nothing and the hasher stays untouched. + #[cfg_attr(not(any(feature = "chargebee", feature = "paypal")), allow(unused_mut))] + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + #[cfg(feature = "chargebee")] + hasher.write_u64(chargebee::TenantChargebee::fingerprint(&chargebee_config)); + #[cfg(feature = "paypal")] + hasher.write_u64(paypal::TenantPaypal::fingerprint(&paypal_config)); + hasher.finish() + }; + // Re-read + fingerprint the company's bound repositories (issue #245): // one index document, read live, so a bind / rotate / revoke reaches the // agent on the next turn. Only companies that explicitly grant `repo` @@ -1132,6 +1189,7 @@ impl HarnessPool { let overlay_fingerprints = self.overlay_fingerprints.read().await; let capability_fingerprints = self.capability_fingerprints.read().await; let composio_fingerprints = self.composio_fingerprints.read().await; + let billing_fingerprints = self.billing_fingerprints.read().await; let repo_fingerprints = self.repo_fingerprints.read().await; let skill_fingerprints = self.skill_fingerprints.read().await; let budget_fingerprints = self.budget_fingerprints.read().await; @@ -1143,6 +1201,7 @@ impl HarnessPool { && overlay_fingerprints.get(&company.id) == Some(&overlay_fp) && capability_fingerprints.get(&company.id) == Some(&capability_fp) && composio_fingerprints.get(&company.id) == Some(&composio_fp) + && billing_fingerprints.get(&company.id) == Some(&billing_fp) && repo_fingerprints.get(&company.id) == Some(&repo_fp) && skill_fingerprints.get(&company.id) == Some(&skill_fp) && budget_fingerprints.get(&company.id) == Some(&budget_fp) @@ -1167,6 +1226,14 @@ impl HarnessPool { // Install the freshly-resolved Composio config the same way, so a token // set/rotate/clear reaches the rebuilt agents (issue #110). fresh_deps.composio = composio_config; + #[cfg(feature = "chargebee")] + { + fresh_deps.chargebee = chargebee_config; + } + #[cfg(feature = "paypal")] + { + fresh_deps.paypal = paypal_config; + } // And the freshly-read bindings (issue #245), so a repository bound or // revoked in the console is what the rebuilt agents' tools resolve // against — including the descriptions that name what is bound. @@ -1229,6 +1296,10 @@ impl HarnessPool { .write() .await .insert(company.id.clone(), composio_fp); + self.billing_fingerprints + .write() + .await + .insert(company.id.clone(), billing_fp); self.repo_fingerprints .write() .await @@ -1327,6 +1398,75 @@ impl HarnessPool { } } + /// Re-reads the company's Chargebee connection from the secret store, so a + /// key saved or rotated in Settings → Billing reaches the agent on its next + /// turn rather than at the next restart (issue #788). + /// + /// Only companies that **explicitly** grant `chargebee` read at all. With no + /// secret store wired this keeps the boot-resolved + /// [`HarnessDeps::chargebee`] — which was itself resolved from *this* + /// company's secret store by the runtime builder, so the fallback cannot + /// reach another tenant's credential. + /// + /// A transient **read error** keeps that connection too, with a warning, + /// rather than un-wiring the billing tools — the same direction + /// [`Self::resolve_repo_bindings`] and [`Self::resolve_effective_mcp`] + /// degrade in, and the safe one here for a specific reason: a stale + /// Chargebee credential is refused by Chargebee, which the agent surfaces as + /// a tool error it can report, whereas a tool that has vanished is invisible + /// to the agent — it simply stops being able to invoice and says nothing. + /// An absent credential still resolves to `None`; only the error case holds. + #[cfg(feature = "chargebee")] + async fn resolve_chargebee( + &self, + company: &CompanyRecord, + deps: &HarnessDeps, + ) -> Option { + if !crate::company::grants_chargebee_explicit(&company.manifest.tools.allow) { + return None; + } + let Some(secrets) = &deps.secrets else { + return deps.chargebee.clone(); + }; + match chargebee::TenantChargebee::resolve(secrets, &company.id).await { + Ok(resolved) => resolved, + Err(err) => { + tracing::warn!( + company = %company.id, + "[chargebee] could not read the billing credential; keeping the last known \ + connection: {err}" + ); + deps.chargebee.clone() + } + } + } + + /// The PayPal equivalent (issue #789), for the same reasons. + #[cfg(feature = "paypal")] + async fn resolve_paypal( + &self, + company: &CompanyRecord, + deps: &HarnessDeps, + ) -> Option { + if !crate::company::grants_paypal_explicit(&company.manifest.tools.allow) { + return None; + } + let Some(secrets) = &deps.secrets else { + return deps.paypal.clone(); + }; + match paypal::TenantPaypal::resolve(secrets, &company.id).await { + Ok(resolved) => resolved, + Err(err) => { + tracing::warn!( + company = %company.id, + "[paypal] could not read the billing credential; keeping the last known \ + connection: {err}" + ); + deps.paypal.clone() + } + } + } + /// Re-reads the company's bound repositories (issue #245) from the /// [`RepoManager`](crate::runtime::RepoManager), so a bind, a credential /// rotation or a revoke reaches the roster on the next turn. @@ -1546,6 +1686,15 @@ impl HarnessPool { self.budget_fingerprints.read().await.get(company).copied() } + /// The current billing-connection fingerprint for a company (test-only), so + /// a credential-freshness test can assert the roster was rebuilt after a key + /// was saved or rotated in Settings → Billing rather than inferring it from + /// the tool list (issues #788, #789). + #[cfg(test)] + pub async fn billing_fingerprint_of(&self, company: &CompanyId) -> Option { + self.billing_fingerprints.read().await.get(company).copied() + } + /// The current desk-scope fingerprint for a company (test-only), so a /// desk-scoping test can assert the roster was actually rebuilt after a /// ceiling or seating change rather than inferring it from a refused call. @@ -3049,6 +3198,10 @@ description = "Builds the product." plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -3254,6 +3407,10 @@ description = "Builds the product." plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -3941,6 +4098,10 @@ description = "Builds the product." plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -4119,6 +4280,10 @@ description = "Builds the product." plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -4370,6 +4535,151 @@ description = "Builds the product." ); } + // --- Billing-credential freshness (issues #788, #789) ------------------- + + /// Saving or rotating a key in Settings → Billing must reach the agent on + /// its next turn. + /// + /// The fingerprint is the observable that makes "no restart" testable: a + /// credential that fails to move it leaves the roster cached, and the agent + /// keeps authenticating with the old key — or holds no billing tools at all + /// — until the process restarts. That failure is invisible from the tool + /// list alone, which is why this asserts the fingerprint directly. + #[tokio::test] + #[cfg(feature = "chargebee")] + async fn ensure_rebuilds_when_a_chargebee_credential_is_saved_or_rotated() { + use crate::chargebee::types::{API_KEY_SECRET, SITE_SECRET}; + + let secrets: Arc = Arc::new(MemSecrets::default()); + let dir = tempfile::tempdir().unwrap(); + let mut deps = deps_with_plan(dir.path(), Arc::new(MockContext::default()), None, None); + deps.secrets = Some(secrets.clone()); + + // The explicit grant is what opens this axis. A `*` wildcard does not + // confer it — see the module docs. + let mut rec = record(); + rec.manifest.tools.allow = vec!["chargebee".to_string()]; + + let write = |key: &'static str, value: &'static str| { + let secrets = secrets.clone(); + async move { + secrets + .set( + &CompanyId::new("acme"), + key, + crate::ports::types::SecretValue(value.to_string()), + ) + .await + .expect("write secret"); + } + }; + + let pool = HarnessPool::new(); + pool.ensure(&rec, &deps).await.expect("first ensure"); + let unset = pool + .billing_fingerprint_of(&rec.id) + .await + .expect("fingerprint"); + + // Stability first, so every change assertion below cannot pass by + // coincidence. + pool.ensure(&rec, &deps).await.expect("redundant ensure"); + assert_eq!( + pool.billing_fingerprint_of(&rec.id).await, + Some(unset), + "an unchanged credential must not move the fingerprint" + ); + + // Half a credential is not a connection, so it must not move either — + // the pair is meaningless apart. + write(SITE_SECRET, "acme-test").await; + pool.ensure(&rec, &deps).await.expect("half ensure"); + assert_eq!( + pool.billing_fingerprint_of(&rec.id).await, + Some(unset), + "a site with no key is still no connection" + ); + + // Connect. + write(API_KEY_SECRET, "cb_first").await; + pool.ensure(&rec, &deps).await.expect("post-connect ensure"); + let connected = pool + .billing_fingerprint_of(&rec.id) + .await + .expect("fingerprint"); + assert_ne!(unset, connected, "saving a credential must rebuild"); + + // Rotate: same site, new key. This is the one a fingerprint over the + // site alone would miss, leaving the agent on the revoked key. + write(API_KEY_SECRET, "cb_rotated").await; + pool.ensure(&rec, &deps).await.expect("post-rotate ensure"); + let rotated = pool + .billing_fingerprint_of(&rec.id) + .await + .expect("fingerprint"); + assert_ne!( + connected, rotated, + "a rotation must rebuild even though the site is identical" + ); + + // Disconnect. + write(API_KEY_SECRET, "").await; + pool.ensure(&rec, &deps).await.expect("post-clear ensure"); + assert_eq!( + pool.billing_fingerprint_of(&rec.id).await, + Some(unset), + "clearing the key must land back on the unconnected fingerprint" + ); + assert_eq!( + pool.resident_companies().await, + 1, + "same company, rebuilt in place — not a new residency" + ); + } + + /// A company that does not explicitly grant `chargebee` never reads the + /// billing secrets, so this axis is inert for it — and a credential sitting + /// in its store confers nothing. Fail closed, as the module docs promise. + #[tokio::test] + #[cfg(feature = "chargebee")] + async fn a_company_without_the_chargebee_grant_never_moves_on_this_axis() { + use crate::chargebee::types::{API_KEY_SECRET, SITE_SECRET}; + + let secrets: Arc = Arc::new(MemSecrets::default()); + let dir = tempfile::tempdir().unwrap(); + let mut deps = deps_with_plan(dir.path(), Arc::new(MockContext::default()), None, None); + deps.secrets = Some(secrets.clone()); + + // A wildcard, deliberately: it must NOT confer billing. + let mut rec = record(); + rec.manifest.tools.allow = vec!["*".to_string()]; + + let pool = HarnessPool::new(); + pool.ensure(&rec, &deps).await.expect("first ensure"); + let before = pool + .billing_fingerprint_of(&rec.id) + .await + .expect("fingerprint"); + + for (key, value) in [(SITE_SECRET, "acme-test"), (API_KEY_SECRET, "cb_key")] { + secrets + .set( + &CompanyId::new("acme"), + key, + crate::ports::types::SecretValue(value.to_string()), + ) + .await + .expect("write secret"); + } + + pool.ensure(&rec, &deps).await.expect("post-write ensure"); + assert_eq!( + pool.billing_fingerprint_of(&rec.id).await, + Some(before), + "an ungranted company must not read the billing secrets, let alone rebuild on them" + ); + } + // --- Skill-delta freshness (issue #41) ---------------------------------- /// An in-memory `SkillStateStore` whose delta set a test can mutate between @@ -4637,6 +4947,10 @@ description = "Builds the product." plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -4809,6 +5123,10 @@ description = "Sets direction." plan: Some(plan), media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, @@ -4961,6 +5279,10 @@ description = "Sets direction." plan, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, artifacts: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), @@ -5918,4 +6240,234 @@ budget_usd_daily = 0.0 } assert!(checked > 0, "no executable tool was on the belt to check"); } + + // --- Per-company billing resolution (issues #788, #789) ----------------- + // + // `resolve_chargebee` / `resolve_paypal` are what actually decide whether a + // company's agents get billing tools on a given turn — `HarnessPool::ensure` + // re-resolves them every turn, and `RuntimeBuilder::build` runs the same + // three-way decision once at boot. All three branches are silent when they + // go wrong: a dropped grant check wires tools the manifest never allowed, and + // a read error collapsed into "no credential" disconnects a working + // integration on one transient store hiccup. + + /// A secret store that reads back what was seeded, or fails every read. + #[cfg(any(feature = "chargebee", feature = "paypal"))] + #[derive(Default)] + struct BillingSecrets { + map: StdMutex>, + fail: bool, + } + + #[cfg(any(feature = "chargebee", feature = "paypal"))] + #[async_trait] + impl SecretStore for BillingSecrets { + async fn get( + &self, + _c: &CompanyId, + key: &str, + ) -> crate::Result> { + if self.fail { + return Err(crate::error::OpenCompanyError::Store( + "the secret store is unreachable".into(), + )); + } + Ok(self + .map + .lock() + .unwrap() + .get(key) + .map(|v| crate::ports::types::SecretValue(v.clone()))) + } + async fn set( + &self, + _c: &CompanyId, + key: &str, + value: crate::ports::types::SecretValue, + ) -> crate::Result<()> { + self.map.lock().unwrap().insert(key.to_string(), value.0); + Ok(()) + } + } + + /// A company whose manifest allows exactly `grants`. + #[cfg(any(feature = "chargebee", feature = "paypal"))] + fn record_granting(grants: &[&str]) -> CompanyRecord { + let mut rec = record(); + rec.manifest.tools.allow = grants.iter().map(|g| g.to_string()).collect(); + rec + } + + /// The inert fixture deps, with a secret store and a "last known" connection. + #[cfg(any(feature = "chargebee", feature = "paypal"))] + fn billing_deps(dir: &std::path::Path, secrets: Arc) -> HarnessDeps { + let mut deps = deps_with_plan(dir, Arc::new(MockContext::default()), None, None); + deps.secrets = Some(secrets); + deps + } + + #[cfg(feature = "chargebee")] + #[tokio::test] + async fn chargebee_resolves_only_for_a_company_that_grants_it() { + let dir = tempfile::tempdir().expect("tempdir"); + let secrets = Arc::new(BillingSecrets::default()); + secrets + .set( + &CompanyId::new("acme"), + crate::chargebee::types::SITE_SECRET, + crate::ports::types::SecretValue("acme-test".into()), + ) + .await + .expect("seed"); + secrets + .set( + &CompanyId::new("acme"), + crate::chargebee::types::API_KEY_SECRET, + crate::ports::types::SecretValue("cb_key".into()), + ) + .await + .expect("seed"); + let deps = billing_deps(dir.path(), secrets); + let pool = HarnessPool::new(); + + // Granted and configured: the credential resolves. + let granted = pool + .resolve_chargebee(&record_granting(&["chargebee"]), &deps) + .await + .expect("a granted, configured company resolves"); + assert_eq!(granted.site(), "acme-test"); + + // Same credentials, no grant. The store is untouched — the gate is the + // manifest, so a company that never opted in gets no tools however well + // configured the host happens to be. + assert!( + pool.resolve_chargebee(&record_granting(&[]), &deps) + .await + .is_none(), + "an ungranted company must resolve nothing" + ); + + // And a wildcard is not a grant: these tools send invoices to real + // people, so they are opted into by name rather than riding in on the + // `*` somebody set for file and shell tools. + assert!( + pool.resolve_chargebee(&record_granting(&["*"]), &deps) + .await + .is_none(), + "a catch-all grant must not confer chargebee" + ); + } + + #[cfg(feature = "chargebee")] + #[tokio::test] + async fn a_chargebee_store_hiccup_keeps_the_last_known_connection() { + // The distinction this pins: absence wires no tools, but a READ FAILURE + // keeps whatever was already resolved. Collapsing the two would drop a + // working company's billing tools mid-conversation on one bad read, and + // silently — the agent would simply stop being able to invoice. + let dir = tempfile::tempdir().expect("tempdir"); + let secrets = Arc::new(BillingSecrets { + fail: true, + ..Default::default() + }); + let mut deps = billing_deps(dir.path(), secrets); + let last_known = crate::harness::chargebee::TenantChargebee::resolve( + &(Arc::new(BillingSecrets { + map: StdMutex::new( + [ + ( + crate::chargebee::types::SITE_SECRET.to_string(), + "acme-test".to_string(), + ), + ( + crate::chargebee::types::API_KEY_SECRET.to_string(), + "cb_key".to_string(), + ), + ] + .into_iter() + .collect(), + ), + fail: false, + }) as Arc), + &CompanyId::new("acme"), + ) + .await + .expect("the seeded store reads") + .expect("both halves present"); + deps.chargebee = Some(last_known); + + let kept = pool_resolve_chargebee(&deps).await; + assert_eq!( + kept.map(|c| c.site().to_string()).as_deref(), + Some("acme-test"), + "a transient read failure must not disconnect a working integration" + ); + } + + #[cfg(feature = "chargebee")] + async fn pool_resolve_chargebee( + deps: &HarnessDeps, + ) -> Option { + HarnessPool::new() + .resolve_chargebee(&record_granting(&["chargebee"]), deps) + .await + } + + #[cfg(feature = "paypal")] + #[tokio::test] + async fn paypal_resolves_only_for_a_company_that_grants_it() { + let dir = tempfile::tempdir().expect("tempdir"); + let secrets = Arc::new(BillingSecrets::default()); + for (key, value) in [ + (crate::company::paypal::CLIENT_ID_SECRET, "AY_id"), + (crate::company::paypal::CLIENT_SECRET_SECRET, "EL_secret"), + ] { + secrets + .set( + &CompanyId::new("acme"), + key, + crate::ports::types::SecretValue(value.into()), + ) + .await + .expect("seed"); + } + let deps = billing_deps(dir.path(), secrets); + let pool = HarnessPool::new(); + + assert!( + pool.resolve_paypal(&record_granting(&["paypal"]), &deps) + .await + .is_some(), + "a granted, configured company resolves" + ); + assert!( + pool.resolve_paypal(&record_granting(&[]), &deps) + .await + .is_none(), + "an ungranted company must resolve nothing" + ); + assert!( + pool.resolve_paypal(&record_granting(&["*"]), &deps) + .await + .is_none(), + "a catch-all grant must not confer paypal" + ); + } + + #[cfg(feature = "paypal")] + #[tokio::test] + async fn a_paypal_grant_with_no_credential_wires_nothing_rather_than_failing() { + // Fail closed: a manifest that grants `paypal` on a host where nobody + // has saved a credential must wire no tools, not tools that fail on + // first use — an agent that HAS a wallet tool tells the operator the + // balance is unavailable, rather than that it cannot read wallets. + let dir = tempfile::tempdir().expect("tempdir"); + let deps = billing_deps(dir.path(), Arc::new(BillingSecrets::default())); + assert!( + HarnessPool::new() + .resolve_paypal(&record_granting(&["paypal"]), &deps) + .await + .is_none() + ); + } } diff --git a/src/harness/paypal.rs b/src/harness/paypal.rs new file mode 100644 index 000000000..82ff62f9c --- /dev/null +++ b/src/harness/paypal.rs @@ -0,0 +1,399 @@ +//! The agent-facing bridge for PayPal (issue #789): two read tools over +//! [`crate::paypal::api`]. +//! +//! Same shape as [`crate::harness::chargebee`] — per-company credentials from +//! that company's [`SecretStore`], resolved at roster-build time, wired only on +//! an explicit `paypal` grant and only when a credential resolves. +//! +//! # Both tools are read-only, and that is the whole surface +//! +//! #789 lists `send_payment` as optional and requires a scoping decision before +//! implementation, so nothing here moves money. Both tools are therefore +//! [`PermissionLevel::ReadOnly`] and never park: asking what the balance is +//! should not need an approval click, and there is no write to guard. + +use std::sync::Arc; + +use crate::company::paypal::{ + CLIENT_ID_SECRET, CLIENT_SECRET_SECRET, ENVIRONMENT_SECRET, PaypalEnvironment, +}; +use crate::ports::SecretStore; +use crate::ports::types::CompanyId; + +/// One company's resolved PayPal connection. +#[derive(Clone, Debug)] +pub struct TenantPaypal { + #[cfg_attr(not(feature = "paypal"), allow(dead_code))] + config: crate::paypal::PaypalConfig, +} + +impl TenantPaypal { + /// Resolves a company's PayPal credentials from its secret store. + /// + /// `Ok(None)` unless BOTH halves are present: a client id with no secret + /// cannot obtain a token, and half a credential should wire no tools rather + /// than tools that fail on first use. + /// + /// A store **read failure** is an `Err`, not `Ok(None)` — see + /// [`crate::harness::chargebee::TenantChargebee::resolve`] for why the two + /// must stay distinguishable. + pub async fn resolve( + secrets: &Arc, + company: &CompanyId, + ) -> crate::error::Result> { + let read = async |key: &str| -> crate::error::Result> { + Ok(secrets + .get(company, key) + .await? + .map(|value| value.0.trim().to_string()) + .filter(|value| !value.is_empty())) + }; + let (Some(client_id), Some(client_secret)) = ( + read(CLIENT_ID_SECRET).await?, + read(CLIENT_SECRET_SECRET).await?, + ) else { + return Ok(None); + }; + // An unset environment is sandbox, matching `PaypalEnvironment::parse`: + // the safe default is reading fake money, never moving real money. + let environment = read(ENVIRONMENT_SECRET) + .await? + .map(|raw| PaypalEnvironment::parse(&raw)) + .unwrap_or_default(); + + Ok(Some(Self { + config: crate::paypal::PaypalConfig { + client_id, + client_secret, + environment, + }, + })) + } + + /// Which PayPal environment this company is pointed at. Never the credential. + pub fn environment(&self) -> PaypalEnvironment { + self.config.environment + } + + /// A stable hash of the connection, for the roster staleness check. + /// + /// Covers the environment as well as both halves of the credential: moving + /// a company from sandbox to live with the same keys must rebuild, or its + /// agents keep reading the wrong world's balance until a restart. + pub fn fingerprint(config: &Option) -> u64 { + use std::hash::{Hash, Hasher}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + match config { + None => 0u8.hash(&mut hasher), + Some(c) => { + 1u8.hash(&mut hasher); + c.config.client_id.hash(&mut hasher); + c.config.client_secret.hash(&mut hasher); + c.config.environment.as_str().hash(&mut hasher); + } + } + hasher.finish() + } +} + +#[cfg(feature = "paypal")] +pub use live::paypal_tools; + +#[cfg(feature = "paypal")] +mod live { + use super::*; + + use anyhow::Result; + use async_trait::async_trait; + use serde_json::{Value, json}; + + use crate::paypal::api; + use crate::paypal::client::PaypalClient; + + use oh::tools::traits::{PermissionLevel, Tool, ToolResult}; + use openhuman_core::openhuman as oh; + + /// Builds the per-company PayPal tools over a resolved connection. + pub fn paypal_tools(config: &TenantPaypal) -> Vec> { + let config = Arc::new(config.clone()); + vec![ + Box::new(WalletBalanceTool(Arc::clone(&config))), + Box::new(ListTransactionsTool(config)), + ] + } + + /// Builds the client for a call about to be made. + fn client(config: &TenantPaypal) -> crate::error::Result { + PaypalClient::new(config.config.clone()) + } + + /// Renders a result, or the failure as text the agent can act on. + fn render(what: &str, outcome: crate::error::Result) -> ToolResult { + match outcome { + Ok(value) => match serde_json::to_string_pretty(&value) { + Ok(text) => ToolResult::success(text), + Err(e) => { + ToolResult::error(format!("{what} succeeded but could not be rendered: {e}")) + } + }, + Err(e) => ToolResult::error(format!("{what} failed: {e}")), + } + } + + pub struct WalletBalanceTool(Arc); + + #[async_trait] + impl Tool for WalletBalanceTool { + fn name(&self) -> &str { + "paypal_get_wallet_balance" + } + + fn description(&self) -> &str { + "Fetch the current PayPal account balance, per currency. Returns the available and \ + withheld amounts as exact decimal strings — report them verbatim rather than \ + rounding or recomputing." + } + + fn parameters_schema(&self) -> Value { + json!({"type": "object", "additionalProperties": false, "properties": {}}) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + async fn execute(&self, _args: Value) -> Result { + let client = match client(&self.0) { + Ok(client) => client, + Err(e) => return Ok(ToolResult::error(format!("paypal client: {e}"))), + }; + tracing::info!( + environment = self.0.environment().as_str(), + "[paypal] get_wallet_balance" + ); + Ok(render( + "paypal_get_wallet_balance", + api::get_wallet_balance(&client).await, + )) + } + } + + pub struct ListTransactionsTool(Arc); + + #[async_trait] + impl Tool for ListTransactionsTool { + fn name(&self) -> &str { + "paypal_list_transactions" + } + + fn description(&self) -> &str { + "List PayPal transactions between two dates. PayPal publishes on a delay of up to 3 \ + hours, so a window ENDING today is fine but one STARTING today usually has no data \ + and is rejected — start at least one day back. The window must span no more than 31 \ + days. To answer 'was I paid recently?', ask for the last 7 days rather than today." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "required": ["start_date", "end_date"], + "additionalProperties": false, + "properties": { + "start_date": { + "type": "string", + "description": "ISO 8601, e.g. 2026-08-01T00:00:00Z. At most 31 days before end_date." + }, + "end_date": { + "type": "string", + "description": "ISO 8601, e.g. 2026-08-13T23:59:59Z." + }, + "page_size": {"type": "integer", "minimum": 1, "maximum": 500} + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + PermissionLevel::ReadOnly + } + + async fn execute(&self, args: Value) -> Result { + let client = match client(&self.0) { + Ok(client) => client, + Err(e) => return Ok(ToolResult::error(format!("paypal client: {e}"))), + }; + let start = args.get("start_date").and_then(Value::as_str).unwrap_or(""); + let end = args.get("end_date").and_then(Value::as_str).unwrap_or(""); + let page_size = args.get("page_size").and_then(Value::as_i64); + Ok(render( + "paypal_list_transactions", + api::list_transactions(&client, start, end, page_size).await, + )) + } + } +} + +#[cfg(all(test, feature = "paypal"))] +mod tests { + use super::*; + use crate::ports::types::SecretValue; + use crate::store::fs::FsSecretStore; + + async fn secrets_with(entries: &[(&str, &str)]) -> (Arc, CompanyId) { + let dir = tempfile::tempdir().expect("tempdir"); + let store: Arc = Arc::new(FsSecretStore::new(dir.keep())); + let company = CompanyId::new("acme"); + for (key, value) in entries { + store + .set(&company, key, SecretValue(value.to_string())) + .await + .expect("set"); + } + (store, company) + } + + #[tokio::test] + async fn both_halves_of_the_credential_are_required() { + let (id_only, company) = secrets_with(&[(CLIENT_ID_SECRET, "AY_id")]).await; + assert!( + TenantPaypal::resolve(&id_only, &company) + .await + .expect("a readable store is not an error") + .is_none() + ); + + let (secret_only, company) = secrets_with(&[(CLIENT_SECRET_SECRET, "EL_secret")]).await; + assert!( + TenantPaypal::resolve(&secret_only, &company) + .await + .expect("a readable store is not an error") + .is_none() + ); + + let (neither, company) = secrets_with(&[]).await; + assert!( + TenantPaypal::resolve(&neither, &company) + .await + .expect("a readable store is not an error") + .is_none() + ); + } + + #[test] + fn the_fingerprint_moves_on_the_credential_and_on_the_environment() { + // Symmetric with the Chargebee side, plus the environment: moving a + // company from sandbox to live with the same keys must rebuild, or its + // agents keep reading the wrong world's balance. + let of = |id: &str, secret: &str, env: PaypalEnvironment| { + TenantPaypal::fingerprint(&Some(TenantPaypal { + config: crate::paypal::PaypalConfig { + client_id: id.to_string(), + client_secret: secret.to_string(), + environment: env, + }, + })) + }; + + let base = of("AY_id", "EL_secret", PaypalEnvironment::Sandbox); + assert_eq!( + base, + of("AY_id", "EL_secret", PaypalEnvironment::Sandbox), + "stable for one config" + ); + assert_ne!( + base, + of("AY_other", "EL_secret", PaypalEnvironment::Sandbox) + ); + assert_ne!(base, of("AY_id", "EL_rotated", PaypalEnvironment::Sandbox)); + assert_ne!( + base, + of("AY_id", "EL_secret", PaypalEnvironment::Live), + "the environment must count on its own" + ); + assert_ne!(base, TenantPaypal::fingerprint(&None)); + } + + #[tokio::test] + async fn an_unset_environment_resolves_to_sandbox() { + // The safe default, and the one that matters most: an operator who never + // touched the environment field must not be reading a live balance. + let (store, company) = secrets_with(&[ + (CLIENT_ID_SECRET, "AY_id"), + (CLIENT_SECRET_SECRET, "EL_secret"), + ]) + .await; + let resolved = TenantPaypal::resolve(&store, &company) + .await + .expect("the store reads") + .expect("both halves present"); + assert_eq!(resolved.environment(), PaypalEnvironment::Sandbox); + } + + #[tokio::test] + async fn live_is_reached_only_by_saying_live() { + let (store, company) = secrets_with(&[ + (CLIENT_ID_SECRET, "AY_id"), + (CLIENT_SECRET_SECRET, "EL_secret"), + (ENVIRONMENT_SECRET, "live"), + ]) + .await; + let resolved = TenantPaypal::resolve(&store, &company) + .await + .expect("the store reads") + .expect("resolves"); + assert_eq!(resolved.environment(), PaypalEnvironment::Live); + + // And a near-miss does not. + let (typo, company) = secrets_with(&[ + (CLIENT_ID_SECRET, "AY_id"), + (CLIENT_SECRET_SECRET, "EL_secret"), + (ENVIRONMENT_SECRET, "Live-ish"), + ]) + .await; + let resolved = TenantPaypal::resolve(&typo, &company) + .await + .expect("the store reads") + .expect("resolves"); + assert_eq!(resolved.environment(), PaypalEnvironment::Sandbox); + } + + #[tokio::test] + async fn the_credential_never_reaches_a_debug_rendering() { + let (store, company) = secrets_with(&[ + (CLIENT_ID_SECRET, "AY_id"), + (CLIENT_SECRET_SECRET, "EL_secret"), + ]) + .await; + let resolved = TenantPaypal::resolve(&store, &company) + .await + .expect("the store reads") + .expect("resolves"); + let rendered = format!("{resolved:?}"); + assert!(!rendered.contains("EL_secret"), "{rendered}"); + assert!(!rendered.contains("AY_id"), "{rendered}"); + } + + #[test] + fn both_tools_are_read_only() { + use oh::tools::traits::PermissionLevel; + use openhuman_core::openhuman as oh; + + let config = TenantPaypal { + config: crate::paypal::PaypalConfig { + client_id: "AY_id".to_string(), + client_secret: "EL_secret".to_string(), + environment: PaypalEnvironment::Sandbox, + }, + }; + let tools = live::paypal_tools(&config); + assert_eq!(tools.len(), 2); + for tool in &tools { + // Nothing here moves money (see the module docs), so nothing parks. + assert_eq!( + tool.permission_level(), + PermissionLevel::ReadOnly, + "{}", + tool.name() + ); + } + } +} diff --git a/src/harness/publish_turn_test.rs b/src/harness/publish_turn_test.rs index 19d958fe9..f72db5edc 100644 --- a/src/harness/publish_turn_test.rs +++ b/src/harness/publish_turn_test.rs @@ -338,6 +338,10 @@ fn brain_with( plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, diff --git a/src/harness/search_turn_test.rs b/src/harness/search_turn_test.rs index 5bbd89b36..109df14fe 100644 --- a/src/harness/search_turn_test.rs +++ b/src/harness/search_turn_test.rs @@ -299,6 +299,10 @@ async fn harness( plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, diff --git a/src/harness/workflow_build/test.rs b/src/harness/workflow_build/test.rs index f71c2e3dd..d44df1644 100644 --- a/src/harness/workflow_build/test.rs +++ b/src/harness/workflow_build/test.rs @@ -710,6 +710,10 @@ pub(crate) fn agent_deps( plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, search: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), diff --git a/src/harness/workspace_provision_turn_test.rs b/src/harness/workspace_provision_turn_test.rs index 705263a3b..34112d0b0 100644 --- a/src/harness/workspace_provision_turn_test.rs +++ b/src/harness/workspace_provision_turn_test.rs @@ -291,6 +291,10 @@ fn build_brain( plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, diff --git a/src/harness/workspace_turn_test.rs b/src/harness/workspace_turn_test.rs index 2e672c3db..4a00c1832 100644 --- a/src/harness/workspace_turn_test.rs +++ b/src/harness/workspace_turn_test.rs @@ -302,6 +302,10 @@ async fn harness( plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: None, diff --git a/src/lib.rs b/src/lib.rs index 1630c6c36..61bb22600 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,6 +6,10 @@ pub mod app; pub mod brain; +/// Chargebee billing (issue #788): the REST client and the billing operations +/// the agent's tools call. The toolbelt bridge lives in `harness::chargebee`. +#[cfg(feature = "chargebee")] +pub mod chargebee; pub mod company; /// Local-only runtime host used by the packaged Tauri desktop application. /// It embeds the existing operator API and ships the curated company presets; @@ -24,6 +28,9 @@ pub mod harness; /// (usage samples, ledger, `[budget]`). No I/O; WS2 wraps these in GraphQL. pub mod metering; pub mod openhuman; +/// PayPal wallet + transaction visibility (issue #789). +#[cfg(feature = "paypal")] +pub mod paypal; pub mod policy; pub mod ports; /// The `x-sdk-name: opencompany` identity attached to this crate's own diff --git a/src/paypal/api.rs b/src/paypal/api.rs new file mode 100644 index 000000000..786b5341b --- /dev/null +++ b/src/paypal/api.rs @@ -0,0 +1,525 @@ +//! The two read operations issue #789 scopes: wallet balance and recent +//! transactions. +//! +//! Deliberately read-only. #789 lists `send_payment` as optional and requires a +//! scoping decision before implementation; moving money is not something to +//! ship on an "optional" line in an issue. + +use serde::Serialize; +use serde_json::Value; + +use super::client::PaypalClient; +use crate::error::{OpenCompanyError, Result}; + +/// One currency's balance in the account. +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] +pub struct Balance { + /// ISO 4217 code. + pub currency_code: String, + /// The amount as PayPal reports it — a decimal STRING, e.g. `"4320.50"`. + /// + /// Kept as text rather than parsed into a float: this value is rendered to + /// an operator, and `4320.50` through an `f64` is how a balance acquires a + /// trailing `0000001`. Nothing here does arithmetic on it. + pub available: String, + /// Funds not yet available, same format. + pub withheld: String, + /// Whether this is the account's primary currency. + pub primary: bool, +} + +/// One transaction, projected down from PayPal's very large record. +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] +pub struct Transaction { + /// PayPal's transaction id. + pub id: String, + /// ISO 8601, as PayPal reports it. + pub date: String, + /// Signed decimal string — negative for money leaving the account. + pub amount: String, + /// ISO 4217 code. + pub currency_code: String, + /// `S` success, `P` pending, `V` reversed, `D` denied. + pub status: String, + /// The counterparty's name or email, when PayPal supplies one. + pub counterparty: Option, + /// The payer-supplied note, when present. + pub note: Option, +} + +/// Rewrites PayPal's opaque "Data for the given start date is not available" +/// into something the caller can act on. +/// +/// PayPal serves transaction data on a lag — a completed payment takes up to +/// three hours to appear — and rejects any window whose start is inside that +/// gap. Its own message says only that data "is not available", which reads as +/// "there were no transactions" rather than "ask for an earlier window", so an +/// agent handed it starts guessing at timeframes instead of moving the start +/// date back. That is exactly what happened in testing: a start date of today +/// failed, and the agent asked the operator to pick a different period rather +/// than knowing what to do. +/// +/// Only this one error is rewritten, and the original text is kept alongside the +/// remedy so nothing is hidden. +fn explain_unavailable_window(error: OpenCompanyError) -> OpenCompanyError { + let OpenCompanyError::Paypal { + status, + code, + message, + } = &error + else { + return error; + }; + // Matched on PayPal's specific sentence, not a bare "not available". + // The looser test also caught unrelated failures — "The requested resource + // is not available" is a 404, and answering it with advice about start + // dates sends the agent adjusting timeframes for a problem that has nothing + // to do with them. + if !message + .to_ascii_lowercase() + .contains("data for the given start date is not available") + { + return error; + } + OpenCompanyError::Paypal { + status: *status, + code: code.clone(), + message: format!( + "{message} PayPal publishes transactions on a delay of up to 3 hours, so a window \ + that starts today may have no data yet — retry with a `start_date` at least a day \ + earlier. The window must also span no more than 31 days." + ), + } +} + +fn text(value: Option<&Value>, key: &str) -> Option { + value? + .get(key) + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + .map(str::to_string) +} + +/// Reads a money field PayPal must have supplied, or fails. +/// +/// The same rule the missing-array checks below already apply, one level down: +/// **never invent a number about money.** These fields used to default to +/// `"0.00"`, so a response whose shape drifted — a renamed field, a projection +/// PayPal serves to some accounts, an object that arrives empty — reported a +/// funded wallet as empty and a real payment as a zero-value transaction. That +/// is not a degraded answer, it is a confident wrong one: an agent told the +/// balance is `0.00` says the company has no money, and an operator acts on it. +/// +/// An error says "PayPal's reply did not parse", which is true and which +/// somebody can fix. `field` names the JSON path so the report identifies which +/// part of the shape moved; the reply itself goes to the log, never into the +/// message — same rule as `unparsed_body_message`. +fn money(parent: Option<&Value>, key: &str, field: &str) -> Result { + text(parent, key).ok_or_else(|| { + tracing::warn!( + field, + parent = %parent.map(|value| value.to_string()).unwrap_or_else(|| "null".to_string()) + .chars().take(200).collect::(), + "[paypal] reply carried no amount where one is required" + ); + OpenCompanyError::Paypal { + status: 0, + code: "unexpected_response".to_string(), + message: format!( + "PayPal's reply carried no `{field}`, and this host does not substitute a zero \ + for an amount PayPal did not report. The reply is in the host log." + ), + } + }) +} + +/// Fetches the account's balances. +pub async fn get_wallet_balance(client: &PaypalClient) -> Result> { + let body = client.get("/v1/reporting/balances", &[]).await?; + let balances = body + .get("balances") + .and_then(Value::as_array) + .ok_or_else(|| { + // Logged, not relayed — same rule as the client's + // `unparsed_body_message` and `chargebee::api::require`. + tracing::warn!( + body = %body.to_string().chars().take(200).collect::(), + "[paypal] reply carried no `balances` array" + ); + OpenCompanyError::Paypal { + status: 0, + code: "unexpected_response".to_string(), + message: + "PayPal's reply carried no `balances` array. The reply is in the host log." + .to_string(), + } + })?; + + balances + .iter() + .map(|entry| { + Ok(Balance { + currency_code: text(Some(entry), "currency") + .or_else(|| text(entry.get("total_balance"), "currency_code")) + .unwrap_or_default(), + available: money( + entry.get("available_balance"), + "value", + "balances[].available_balance.value", + )?, + withheld: money( + entry.get("withheld_balance"), + "value", + "balances[].withheld_balance.value", + )?, + primary: entry + .get("primary") + .and_then(Value::as_bool) + .unwrap_or(false), + }) + }) + .collect() +} + +/// Fetches transactions between two ISO 8601 instants. +/// +/// PayPal caps the window at **31 days** and publishes on a lag of up to three +/// hours; both limits are enforced by PayPal, not here. Only the non-empty +/// check below is local — parsing ISO 8601 to pre-validate the span would mean +/// duplicating PayPal's calendar rules to save one round trip, and getting that +/// subtly wrong would reject windows PayPal would have accepted. What this does +/// instead is make PayPal's own refusal actionable: see +/// [`explain_unavailable_window`]. +pub async fn list_transactions( + client: &PaypalClient, + start_date: &str, + end_date: &str, + page_size: Option, +) -> Result> { + if start_date.trim().is_empty() || end_date.trim().is_empty() { + return Err(OpenCompanyError::Paypal { + status: 0, + code: "invalid_arguments".to_string(), + message: "`start_date` and `end_date` are both required, in ISO 8601. PayPal allows a \ + window of at most 31 days and publishes on a delay of up to 3 hours." + .to_string(), + }); + } + + let query = vec![ + ("start_date".to_string(), start_date.trim().to_string()), + ("end_date".to_string(), end_date.trim().to_string()), + // Without this PayPal returns only ids and amounts — no counterparty, + // no note — and the answer reads as a list of anonymous numbers. + ( + "fields".to_string(), + "transaction_info,payer_info".to_string(), + ), + ( + "page_size".to_string(), + page_size.unwrap_or(20).clamp(1, 500).to_string(), + ), + ]; + + let body = client + .get("/v1/reporting/transactions", &query) + .await + .map_err(explain_unavailable_window)?; + let rows = body + .get("transaction_details") + .and_then(Value::as_array) + .ok_or_else(|| { + // A successful empty array means no transactions. A missing array + // means the response shape changed, and reporting the latter as an + // empty history would be a confident lie about money. + tracing::warn!( + body = %body.to_string().chars().take(200).collect::(), + "[paypal] reply carried no `transaction_details` array" + ); + OpenCompanyError::Paypal { + status: 0, + code: "unexpected_response".to_string(), + message: "PayPal's reply carried no `transaction_details` array. The reply is in the host log." + .to_string(), + } + })?; + + rows.iter() + .map(|row| { + let info = row.get("transaction_info"); + let payer = row.get("payer_info"); + Ok(Transaction { + id: text(info, "transaction_id").unwrap_or_default(), + date: text(info, "transaction_initiation_date").unwrap_or_default(), + amount: money( + info.and_then(|i| i.get("transaction_amount")), + "value", + "transaction_details[].transaction_info.transaction_amount.value", + )?, + currency_code: text( + info.and_then(|i| i.get("transaction_amount")), + "currency_code", + ) + .unwrap_or_default(), + status: text(info, "transaction_status").unwrap_or_default(), + counterparty: text( + payer.and_then(|p| p.get("payer_name")), + "alternate_full_name", + ) + .or_else(|| text(payer, "email_address")), + note: text(info, "transaction_note"), + }) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::company::paypal::PaypalEnvironment; + use crate::paypal::client::{PaypalClient, PaypalConfig}; + + /// A client pointed at a stub serving `body` for every PayPal path. + /// + /// The token endpoint answers too, so an operation under test goes through + /// the same auth path it does in production rather than a client with the + /// credential step skipped. Abort the returned handle when done. + async fn stub_client(body: &'static str) -> (PaypalClient, tokio::task::JoinHandle<()>) { + let handler = move |uri: axum::http::Uri| async move { + let payload = if uri.path().contains("oauth2/token") { + r#"{"access_token":"tok","expires_in":32400}"# + } else { + body + }; + ( + axum::http::StatusCode::OK, + [("content-type", "application/json")], + payload, + ) + }; + let app = axum::Router::new().fallback(axum::routing::any(handler)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("addr"); + let server = tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + let client = PaypalClient::with_base_url( + PaypalConfig { + client_id: "AY_id".to_string(), + client_secret: "EL_secret".to_string(), + environment: PaypalEnvironment::Sandbox, + }, + format!("http://{addr}"), + ) + .expect("client builds"); + (client, server) + } + + #[tokio::test] + async fn a_balance_keeps_paypals_decimal_string_verbatim() { + // Through an f64 this becomes 4320.500000000001 on some inputs. It is + // rendered to an operator and never computed on, so it stays text. + // Driven through `get_wallet_balance` itself, against a stub serving + // PayPal's real response shape. An earlier version of this test rebuilt + // the projection inline and asserted on its own copy — which passes + // whatever the production projection does, including not existing. + let (client, server) = stub_client( + r#"{"balances":[{ + "currency":"USD","primary":true, + "available_balance":{"currency_code":"USD","value":"4320.50"}, + "withheld_balance":{"currency_code":"USD","value":"12.30"} + },{ + "currency":"EUR","primary":false, + "available_balance":{"currency_code":"EUR","value":"0.00"}, + "withheld_balance":{"currency_code":"EUR","value":"0.00"} + }]}"#, + ) + .await; + let balances = get_wallet_balance(&client).await.expect("balances"); + server.abort(); + + assert_eq!(balances.len(), 2); + // Through an f64 this becomes 4320.500000000001 on some inputs. It is + // rendered to an operator and never computed on, so it stays text. + assert_eq!(balances[0].available, "4320.50"); + assert_eq!(balances[0].withheld, "12.30"); + assert_eq!(balances[0].currency_code, "USD"); + assert!(balances[0].primary); + assert_eq!(balances[1].currency_code, "EUR"); + assert!(!balances[1].primary); + } + + #[tokio::test] + async fn a_reply_without_a_balances_array_is_an_error_not_an_empty_wallet() { + // An account always has balances, so a reply without them is a broken + // integration — reporting "no funds" would be a confident lie about + // money. The transaction query follows the same rule: an empty array + // is a real empty history, but a missing array is not. + let (client, server) = stub_client(r#"{"name":"INTERNAL","debug_id":"x"}"#).await; + let err = get_wallet_balance(&client) + .await + .expect_err("a missing array is an error"); + server.abort(); + let rendered = err.to_string(); + assert!(rendered.contains("balances"), "{rendered}"); + // And the body itself is logged, not relayed into the transcript. + assert!(!rendered.contains("debug_id"), "{rendered}"); + } + + #[tokio::test] + async fn a_balance_with_no_amount_is_an_error_rather_than_a_fabricated_zero() { + // These fields used to default to "0.00". A response whose shape drifted + // therefore reported a funded wallet as EMPTY — not a degraded answer but + // a confident wrong one, which an agent relays and an operator acts on. + for (label, body) in [ + ( + "no available_balance at all", + r#"{"balances":[{"currency":"USD","primary":true, + "withheld_balance":{"currency_code":"USD","value":"12.30"}}]}"#, + ), + ( + "an available_balance with no value", + r#"{"balances":[{"currency":"USD","primary":true, + "available_balance":{"currency_code":"USD"}, + "withheld_balance":{"currency_code":"USD","value":"12.30"}}]}"#, + ), + ( + "an empty-string value", + r#"{"balances":[{"currency":"USD","primary":true, + "available_balance":{"currency_code":"USD","value":""}, + "withheld_balance":{"currency_code":"USD","value":"12.30"}}]}"#, + ), + ( + "no withheld_balance", + r#"{"balances":[{"currency":"USD","primary":true, + "available_balance":{"currency_code":"USD","value":"4320.50"}}]}"#, + ), + ] { + let (client, server) = stub_client(body).await; + let err = match get_wallet_balance(&client).await { + Ok(balances) => { + panic!("{label}: a missing amount must not become a number: {balances:?}") + } + Err(err) => err, + }; + server.abort(); + let rendered = err.to_string(); + // The report names WHICH part of the shape moved, so the fix is + // findable rather than "PayPal broke". + assert!(rendered.contains("balance.value"), "{label}: {rendered}"); + assert!(!rendered.contains("0.00"), "{label}: {rendered}"); + } + } + + #[tokio::test] + async fn a_transaction_with_no_amount_is_an_error_rather_than_a_zero_payment() { + // Same rule for the transaction list: a payment reported as 0.00 reads + // as a failed or free transaction, which is a lie about money rather + // than a gap in the answer. + let (client, server) = stub_client( + r#"{"transaction_details":[{"transaction_info":{ + "transaction_id":"T1","transaction_status":"S", + "transaction_initiation_date":"2026-08-01T00:00:00+0000" + }}]}"#, + ) + .await; + let err = match list_transactions( + &client, + "2026-08-01T00:00:00Z", + "2026-08-02T00:00:00Z", + None, + ) + .await + { + Ok(rows) => panic!("a missing amount must not become 0.00: {rows:?}"), + Err(err) => err, + }; + server.abort(); + let rendered = err.to_string(); + assert!(rendered.contains("transaction_amount.value"), "{rendered}"); + assert!(!rendered.contains("0.00"), "{rendered}"); + } + + #[tokio::test] + async fn a_reply_without_transaction_details_is_an_error_not_an_empty_history() { + let (client, server) = stub_client(r#"{"name":"INTERNAL","debug_id":"x"}"#).await; + let err = list_transactions( + &client, + "2026-08-01T00:00:00Z", + "2026-08-02T00:00:00Z", + None, + ) + .await + .expect_err("a missing array is an error"); + server.abort(); + let rendered = err.to_string(); + assert!(rendered.contains("transaction_details"), "{rendered}"); + assert!(!rendered.contains("debug_id"), "{rendered}"); + } + + #[test] + fn an_unavailable_window_is_explained_rather_than_relayed() { + // PayPal's own words read as "there were no transactions", which sends + // an agent guessing at timeframes instead of moving the start date back. + let raw = OpenCompanyError::Paypal { + status: 400, + code: "INVALID_REQUEST".to_string(), + message: "Data for the given start date is not available.".to_string(), + }; + let explained = explain_unavailable_window(raw).to_string(); + assert!( + explained.contains("Data for the given start date"), + "{explained}" + ); + assert!(explained.contains("3 hours"), "{explained}"); + assert!(explained.contains("start_date"), "{explained}"); + + // Every other failure passes through untouched — this must not become a + // catch-all that buries unrelated PayPal errors under a date hint. + let other = OpenCompanyError::Paypal { + status: 401, + code: "NOT_AUTHORIZED".to_string(), + message: "Authorization failed due to insufficient permissions.".to_string(), + }; + let untouched = explain_unavailable_window(other).to_string(); + assert!(!untouched.contains("3 hours"), "{untouched}"); + + // And neither does an unrelated failure that merely says "not + // available" — a 404 answered with advice about start dates sends the + // agent adjusting timeframes for a problem that has nothing to do with + // them. + let missing = OpenCompanyError::Paypal { + status: 404, + code: "RESOURCE_NOT_FOUND".to_string(), + message: "The requested resource is not available.".to_string(), + }; + let relayed = explain_unavailable_window(missing).to_string(); + assert!(!relayed.contains("3 hours"), "{relayed}"); + assert!(!relayed.contains("start_date"), "{relayed}"); + } + + #[tokio::test] + async fn a_missing_date_is_rejected_before_any_request() { + use crate::company::paypal::PaypalEnvironment; + use crate::paypal::client::{PaypalClient, PaypalConfig}; + // Port 0 never listens, so anything that reached the network would fail + // fast rather than hang — the rejection must happen before that. + let client = PaypalClient::with_base_url( + PaypalConfig { + client_id: "id".into(), + client_secret: "secret".into(), + environment: PaypalEnvironment::Sandbox, + }, + "http://127.0.0.1:0".into(), + ) + .expect("builds"); + + let err = list_transactions(&client, "", "2026-08-13T00:00:00Z", None) + .await + .expect_err("an empty start is rejected"); + assert!(err.to_string().contains("31 days"), "got: {err}"); + } +} diff --git a/src/paypal/client.rs b/src/paypal/client.rs new file mode 100644 index 000000000..58a93d342 --- /dev/null +++ b/src/paypal/client.rs @@ -0,0 +1,550 @@ +//! A PayPal REST client: OAuth2 client-credentials, with the access token +//! cached for its lifetime. +//! +//! # Why the token is cached +//! +//! PayPal issues a bearer token from `POST /v1/oauth2/token` that lasts about +//! nine hours. Fetching one per call would double every request and, at any +//! volume, run into PayPal's rate limit on the token endpoint specifically — +//! which fails the *next* call rather than the one that caused it. So a token is +//! held until shortly before it expires and then re-fetched. +//! +//! The cache lives on the client, and the client is built per company, so one +//! company's token can never authenticate another's call. + +use std::sync::Arc; +use std::time::{Duration, Instant}; + +use serde_json::Value; +use tokio::sync::Mutex; + +use crate::company::paypal::PaypalEnvironment; +use crate::error::{OpenCompanyError, Result}; + +/// A cached bearer token and when it stops being usable. +// No `Debug`. This holds a live bearer token, and a derived `Debug` prints it +// in full — one `{:?}` in a log line or a panic message is the whole +// credential. `PaypalClient` already implements `Debug` by hand and omits this +// field for the same reason; deriving it here would reopen the hole one level +// down. +#[derive(Clone)] +struct CachedToken { + token: String, + /// When to stop trusting it. Deliberately earlier than PayPal's own expiry + /// — see [`PaypalClient::token`]. + good_until: Instant, +} + +/// One company's PayPal credentials and environment. +#[derive(Clone)] +pub struct PaypalConfig { + /// The REST app client id. + pub client_id: String, + /// The REST app secret. + pub client_secret: String, + /// Which PayPal world these belong to. + pub environment: PaypalEnvironment, +} + +/// Prints the environment and **redacts both halves of the credential**. +/// +/// Same reasoning as `ChargebeeConfig`: this is reachable from large aggregates +/// that debugging code prints wholesale, and a derived `Debug` would put a live +/// PayPal secret into any log line that formatted one. +impl std::fmt::Debug for PaypalConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PaypalConfig") + .field("environment", &self.environment) + .field("client_id", &"") + .field("client_secret", &"") + .finish() + } +} + +/// A PayPal API client bound to one company's credentials. +#[derive(Clone)] +pub struct PaypalClient { + http: reqwest::Client, + config: PaypalConfig, + base_url: String, + cached: Arc>>, +} + +impl std::fmt::Debug for PaypalClient { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PaypalClient") + .field("base_url", &self.base_url) + .finish_non_exhaustive() + } +} + +/// Builds the crate error for a PayPal failure. +fn err(status: u16, code: &str, message: impl Into) -> OpenCompanyError { + OpenCompanyError::Paypal { + status, + code: code.to_string(), + message: message.into(), + } +} + +/// Reports a body PayPal did not describe, WITHOUT putting it in the message. +/// +/// Same rule as the Chargebee client's `unparsed_body_message`, for the same +/// reason: PayPal's own `message` / `error_description` is a classified failure +/// the agent should read, but a body carrying neither is unidentified text on a +/// payments API, and this string reaches the model's context and the turn's +/// durable transcript. +fn unparsed_body_message(status: u16, body: &str) -> String { + tracing::warn!( + status, + body = %body.chars().take(200).collect::(), + "[paypal] response body carried no error description" + ); + format!( + "PayPal returned {status} with a body this host could not interpret. The body is in the \ + host log; it is not reproduced here because its contents are unknown and may carry \ + account data." + ) +} + +/// Checks that `path` is a plain absolute path before anything pastes it onto +/// the base URL. +/// +/// [`PaypalClient::get`] builds its URL by concatenation, and concatenation is +/// not host-safe: `"https://api.paypal.com"` followed by `"@evil.com/v1"` parses +/// as userinfo `api.paypal.com` against host `evil.com`, so the bearer token +/// goes to whoever owns that name. No caller passes a dynamic path today — both +/// call sites in [`crate::paypal::api`] are literals — but `get` is `pub`, this +/// is a payments client, and the obvious next operation takes an id from a tool +/// argument. Making the URL unforgeable here costs a comparison per call and +/// means that caller cannot introduce the hole by accident. +/// +/// `?` and `#` are refused for a different reason: the query is a separate +/// argument that reqwest appends, so a path carrying its own would silently +/// merge with it or truncate it. `//` is not exploitable against a base URL that +/// already carries a scheme and host, but no PayPal endpoint has such a path, +/// and allowing it would mean a reader has to re-derive the URL grammar to +/// convince themselves of that. +fn check_path(path: &str) -> Result<()> { + let rejected = !path.starts_with('/') + || path.starts_with("//") + || path.contains('@') + || path.contains('?') + || path.contains('#') + || path.chars().any(|c| c.is_whitespace() || c.is_control()); + if rejected { + return Err(err( + 0, + "invalid_path", + // The path itself is not echoed: it is the untrusted half of this + // check, and this message reaches the model's context. + "refusing to build a PayPal request from a path that is not a plain absolute path", + )); + } + Ok(()) +} + +impl PaypalClient { + /// Builds a client against the environment named in `config`. + pub fn new(config: PaypalConfig) -> Result { + let base = config.environment.base_url().to_string(); + Self::with_base_url(config, base) + } + + /// Builds a client against an explicit base URL, with no trailing slash. + pub fn with_base_url(config: PaypalConfig, base_url: String) -> Result { + let http = reqwest::Client::builder() + .timeout(Duration::from_secs(30)) + // The token call carries HTTP Basic and every other call a bearer; + // reqwest re-sends both across redirects, so a 30x to `http://` + // would leak them. PayPal's API does not redirect. + .redirect(reqwest::redirect::Policy::none()) + .build() + .map_err(|e| err(0, "client_build_failed", e.to_string()))?; + Ok(Self { + http, + config, + base_url: base_url.trim_end_matches('/').to_string(), + cached: Arc::new(Mutex::new(None)), + }) + } + + /// A usable bearer token, from cache when one is still good. + async fn token(&self) -> Result { + let mut slot = self.cached.lock().await; + if let Some(cached) = slot.as_ref() + && Instant::now() < cached.good_until + { + return Ok(cached.token.clone()); + } + + let response = self + .http + .post(format!("{}/v1/oauth2/token", self.base_url)) + .basic_auth(&self.config.client_id, Some(&self.config.client_secret)) + .form(&[("grant_type", "client_credentials")]) + .send() + .await + // `without_url` keeps reqwest's cause — connection refused, TLS + // failure, timeout — and drops the URL it would otherwise print. + // The host is not a secret, but the query string is caller-shaped, + // and this text reaches the model's context and the transcript. + .map_err(|e| err(0, "transport_error", e.without_url().to_string()))?; + + let status = response.status().as_u16(); + let body = response + .text() + .await + .map_err(|e| err(status, "unreadable_body", e.to_string()))?; + let parsed: Value = serde_json::from_str(&body).unwrap_or(Value::Null); + + if !(200..300).contains(&status) { + // PayPal answers bad credentials with `invalid_client`, which reads + // like a typo. Naming the environment turns the commonest actual + // cause — sandbox keys against live, or the reverse — into + // something an operator can see. + return Err(err( + status, + parsed + .get("error") + .and_then(Value::as_str) + .unwrap_or("token_failed"), + format!( + "could not obtain a PayPal token for the `{}` environment: {}", + self.config.environment.as_str(), + parsed + .get("error_description") + .and_then(Value::as_str) + .map(str::to_string) + .unwrap_or_else(|| unparsed_body_message(status, &body)) + ), + )); + } + + let token = parsed + .get("access_token") + .and_then(Value::as_str) + .ok_or_else(|| { + err( + status, + "unexpected_response", + "no `access_token` in the reply", + ) + })? + .to_string(); + + // Re-fetch before PayPal's own expiry rather than at it: a token that + // expires mid-flight fails the call that carried it, and a minute of + // margin costs nothing against a nine-hour lifetime. + let lifetime = token_lifetime( + parsed + .get("expires_in") + .and_then(Value::as_u64) + .unwrap_or(300), + ); + *slot = Some(CachedToken { + token: token.clone(), + good_until: Instant::now() + Duration::from_secs(lifetime), + }); + Ok(token) + } + + /// `GET path` with query parameters, returning the decoded JSON. + /// + /// `path` must be a plain absolute path — see [`check_path`]. It is checked + /// before the token is fetched, so a rejected path costs no round trip and, + /// more to the point, never puts a credential on the wire. + pub async fn get(&self, path: &str, query: &[(String, String)]) -> Result { + check_path(path)?; + let token = self.token().await?; + let response = self + .http + .get(format!("{}{path}", self.base_url)) + .bearer_auth(token) + .query(query) + .send() + .await + // `without_url` keeps reqwest's cause — connection refused, TLS + // failure, timeout — and drops the URL it would otherwise print. + // The host is not a secret, but the query string is caller-shaped, + // and this text reaches the model's context and the transcript. + .map_err(|e| err(0, "transport_error", e.without_url().to_string()))?; + + let status = response.status().as_u16(); + let body = response + .text() + .await + .map_err(|e| err(status, "unreadable_body", e.to_string()))?; + let parsed: Value = serde_json::from_str(&body).unwrap_or(Value::Null); + + if (200..300).contains(&status) { + // A success whose body is not a JSON object is not a success we can + // use: `Value::Null` flows on and every field read yields a default, + // so a proxy's HTML 200 would read as an account with no balances + // rather than a reported failure. Same rule as the Chargebee client. + if !parsed.is_object() { + return Err(err( + status, + "unexpected_response", + unparsed_body_message(status, &body), + )); + } + return Ok(parsed); + } + Err(err( + status, + parsed + .get("name") + .and_then(Value::as_str) + .unwrap_or("unknown"), + parsed + .get("message") + .and_then(Value::as_str) + .map(str::to_string) + .unwrap_or_else(|| unparsed_body_message(status, &body)), + )) + } +} + +/// How long to trust a token PayPal says lasts `expires_in` seconds. +/// +/// Re-fetch before PayPal's own expiry rather than at it: a token that expires +/// mid-flight fails the call that carried it, and a minute of margin costs +/// nothing against a nine-hour lifetime. +/// +/// The floor must not outlive the token it is a floor for. On a ten-second +/// `expires_in`, `saturating_sub(60)` is 0 and a bare `.max(30)` would cache a +/// credential three times longer than it is valid — handing out a dead token, +/// which is the exact failure the margin exists to prevent. +fn token_lifetime(expires_in: u64) -> u64 { + expires_in.saturating_sub(60).max(30).min(expires_in) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A cached token must never outlive the grant it came from. The margin is + /// a subtraction with a floor, and a floor applied to a grant shorter than + /// itself inverts the whole point of the margin. + #[test] + fn a_token_is_never_trusted_past_its_own_expiry() { + // The ordinary case: nine hours, a minute of margin. + assert_eq!(token_lifetime(32400), 32340); + // The margin still applies well above the floor. + assert_eq!(token_lifetime(300), 240); + // Below the margin the floor would run past expiry — clamped instead. + for expires_in in [0, 1, 10, 30, 59, 60, 89] { + assert!( + token_lifetime(expires_in) <= expires_in, + "a {expires_in}s grant was trusted for {}s", + token_lifetime(expires_in), + ); + } + } + + fn config() -> PaypalConfig { + PaypalConfig { + client_id: "AY_client".to_string(), + client_secret: "EL_secret".to_string(), + environment: PaypalEnvironment::Sandbox, + } + } + + #[test] + fn debug_never_renders_either_half_of_the_credential() { + let rendered = format!("{:?}", config()); + assert!(!rendered.contains("AY_client"), "{rendered}"); + assert!(!rendered.contains("EL_secret"), "{rendered}"); + assert!(rendered.contains("Sandbox"), "{rendered}"); + } + + #[test] + fn a_client_debug_does_not_reach_into_its_config() { + let client = PaypalClient::new(config()).expect("builds"); + let rendered = format!("{client:?}"); + assert!(!rendered.contains("EL_secret"), "{rendered}"); + assert!(rendered.contains("sandbox.paypal.com"), "{rendered}"); + } + + #[tokio::test] + async fn a_token_is_reused_rather_than_refetched() { + use std::sync::atomic::{AtomicUsize, Ordering}; + let hits = Arc::new(AtomicUsize::new(0)); + let seen = hits.clone(); + + let app = axum::Router::new().fallback(axum::routing::any(move || { + let seen = seen.clone(); + async move { + seen.fetch_add(1, Ordering::SeqCst); + ( + axum::http::StatusCode::OK, + [("content-type", "application/json")], + r#"{"access_token":"tok_1","expires_in":32400}"#, + ) + } + })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("addr"); + let server = tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let client = + PaypalClient::with_base_url(config(), format!("http://{addr}")).expect("builds"); + assert_eq!(client.token().await.expect("first"), "tok_1"); + assert_eq!(client.token().await.expect("second"), "tok_1"); + assert_eq!(client.token().await.expect("third"), "tok_1"); + // Three calls, ONE trip: without the cache every API call would pay for + // a token, and PayPal rate-limits that endpoint separately. + assert_eq!(hits.load(Ordering::SeqCst), 1); + server.abort(); + } + + #[tokio::test] + async fn bad_credentials_name_the_environment_not_just_invalid_client() { + let app = axum::Router::new().fallback(axum::routing::any(|| async { + ( + axum::http::StatusCode::UNAUTHORIZED, + [("content-type", "application/json")], + r#"{"error":"invalid_client","error_description":"Client Authentication failed"}"#, + ) + })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("addr"); + let server = tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let client = + PaypalClient::with_base_url(config(), format!("http://{addr}")).expect("builds"); + let message = client + .token() + .await + .expect_err("401 is an error") + .to_string(); + // "invalid_client" alone reads as a typo; the commonest real cause is + // sandbox keys pointed at live, which only the environment reveals. + assert!(message.contains("sandbox"), "{message}"); + assert!( + message.contains("Client Authentication failed"), + "{message}" + ); + server.abort(); + } + + #[tokio::test] + async fn a_path_that_could_move_the_host_is_refused_before_any_request() { + use std::sync::atomic::{AtomicUsize, Ordering}; + // A live stub, so "was refused" is proved by the request never arriving + // rather than by an error that a connection failure would also produce. + // The count also covers the TOKEN call: a rejected path must not spend a + // credential fetch either. + let hits = Arc::new(AtomicUsize::new(0)); + let seen = hits.clone(); + let app = axum::Router::new().fallback(axum::routing::any(move || { + let seen = seen.clone(); + async move { + seen.fetch_add(1, Ordering::SeqCst); + ( + axum::http::StatusCode::OK, + [("content-type", "application/json")], + r#"{"access_token":"tok","expires_in":32400,"balances":[]}"#, + ) + } + })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("addr"); + let server = tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let client = + PaypalClient::with_base_url(config(), format!("http://{addr}")).expect("builds"); + + for path in [ + // The one that matters: concatenated onto the base this reads as + // userinfo `api.paypal.com` at host `evil.com`, and the bearer token + // goes to whoever owns that name. + "@evil.com/v1/reporting/balances", + "/v1/reporting/balances@evil.com", + // Not absolute — pastes straight onto the host name. + "v1/reporting/balances", + "evil.com/v1", + // Protocol-relative. + "//evil.com/v1", + // The query is a separate argument; a path carrying its own would + // silently merge with or truncate it. + "/v1/reporting/balances?start_date=x", + "/v1/reporting/balances#frag", + // Header/URL splitting. + "/v1/reporting/ balances", + "/v1/reporting/\nbalances", + ] { + let error = client + .get(path, &[]) + .await + .expect_err(&format!("{path:?} must be refused")); + assert!( + matches!(&error, OpenCompanyError::Paypal { code, .. } if code == "invalid_path"), + "{path:?} was refused, but for the wrong reason: {error}", + ); + // And the path itself is not echoed back into the transcript. + assert!(!error.to_string().contains("evil.com"), "{error}"); + } + + // Nothing reached the network at all — not the request, and not the + // token fetch that would have preceded it. + assert_eq!(hits.load(Ordering::SeqCst), 0); + // The guard rejects; it does not reject everything. A real path still + // goes through, or the check would be indistinguishable from a break. + client + .get("/v1/reporting/balances", &[]) + .await + .expect("a plain absolute path is still allowed"); + assert!(hits.load(Ordering::SeqCst) > 0); + server.abort(); + } + + #[tokio::test] + async fn a_body_paypal_did_not_describe_is_logged_rather_than_relayed() { + // Symmetric with the Chargebee client: PayPal's own `message` is a + // classified failure the agent should read, but a body carrying none is + // unidentified text on a payments API and this string reaches the + // model's context and the durable transcript. + let app = axum::Router::new().fallback(axum::routing::any(|| async { + ( + axum::http::StatusCode::BAD_GATEWAY, + [("content-type", "text/html")], + "upstream error for sb-ml643z@business.example.com, balance 5000.00", + ) + })); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind"); + let addr = listener.local_addr().expect("addr"); + let server = tokio::spawn(async move { + let _ = axum::serve(listener, app).await; + }); + + let client = + PaypalClient::with_base_url(config(), format!("http://{addr}")).expect("builds"); + // The token call fails on the same body, which is the path that runs + // first — both fallbacks share `unparsed_body_message`. + let message = client + .get("/v1/reporting/balances", &[]) + .await + .expect_err("502 is an error") + .to_string(); + assert!(!message.contains("business.example.com"), "{message}"); + assert!(!message.contains("5000.00"), "{message}"); + assert!(message.contains("host log"), "{message}"); + server.abort(); + } +} diff --git a/src/paypal/mod.rs b/src/paypal/mod.rs new file mode 100644 index 000000000..77885b936 --- /dev/null +++ b/src/paypal/mod.rs @@ -0,0 +1,25 @@ +//! PayPal wallet and transaction visibility (issue #789). +//! +//! Read-only by design. #789 lists `send_payment` as optional and requires a +//! scoping decision before implementation; moving money is not something to +//! ship on the strength of an "optional" line. +//! +//! # Relationship to Chargebee (#788) +//! +//! They are complementary, not alternatives. Invoices are created in Chargebee; +//! PayPal is configured as a payment method *inside* Chargebee, so an invoice's +//! payment routes through the connected PayPal account with no code here. What +//! this module adds is the read side — what is in the wallet, and what has +//! moved through it lately. +//! +//! # Credentials +//! +//! Client id + secret, per company, from that company's `SecretStore`. Not the +//! browser OAuth flow an earlier sketch described: that grants access to +//! *someone else's* PayPal account, and these tools read the company's own. +//! There is no third party for a popup to ask. + +pub mod api; +pub mod client; + +pub use client::{PaypalClient, PaypalConfig}; diff --git a/src/policy/consequence.rs b/src/policy/consequence.rs index 75aeefad0..d8b3fa369 100644 --- a/src/policy/consequence.rs +++ b/src/policy/consequence.rs @@ -609,6 +609,42 @@ const DECLARED: &[Declared] = &[ Reach::Nothing, ), d("mcp_call_tool", EffectGroup::Other, Reach::Consequence), + // Billing (issues #788, #789). Both integrations read the company's OWN + // Chargebee site and PayPal account, so the reads are `Nothing` rather than + // `ExternalRead`: that tier exists for reaching into a *counterparty's* + // account, and a `readonly` desk answering "has Alan paid?" about the + // company's own ledger changes nothing and bills nothing. + d("chargebee_get_invoice", EffectGroup::Other, Reach::Nothing), + d( + "chargebee_list_invoices", + EffectGroup::Other, + Reach::Nothing, + ), + d("chargebee_get_customer", EffectGroup::Other, Reach::Nothing), + d( + "paypal_get_wallet_balance", + EffectGroup::Other, + Reach::Nothing, + ), + d( + "paypal_list_transactions", + EffectGroup::Other, + Reach::Nothing, + ), + // Raising an invoice reaches a real customer of a real business and creates + // a demand for money, so it is `Send` and it parks. + d( + "chargebee_send_invoice", + EffectGroup::Send, + Reach::Consequence, + ), + // Writes a record into an external billing system. No money moves, but it + // is still a change somebody else's system will keep. + d( + "chargebee_create_customer", + EffectGroup::Other, + Reach::Consequence, + ), d( "mcp_registry_tool_call", EffectGroup::Other, diff --git a/src/runtime/builder.rs b/src/runtime/builder.rs index efdb9945b..370558fae 100644 --- a/src/runtime/builder.rs +++ b/src/runtime/builder.rs @@ -2094,6 +2094,60 @@ impl RuntimeBuilder { // (fail closed). `HarnessPool::ensure` re-resolves this // each turn so a console token change takes effect // without restart. + // Issue #788: the per-company Chargebee connection, + // resolved from THIS company's secret store — never + // the environment, because two companies on one host + // bill two different sites. Only companies that + // explicitly grant `chargebee` resolve at all; with + // either half of the pair missing it stays `None` + // (fail closed). `HarnessPool::ensure` re-resolves it + // each turn, so a key saved in the console's Billing + // settings takes effect without a restart. + // Issue #789: the per-company PayPal connection, + // resolved from this company's own secret store for + // the same reason chargebee is. + // + // A store read error degrades to `None` HERE, unlike + // in `HarnessPool::resolve_*`, which keeps the last + // known connection: at boot there is no last known + // one to keep. It is warned rather than fatal — + // refusing to start the company over an unreadable + // billing credential would take down every other + // tool it has — and the next turn re-resolves. + #[cfg(feature = "paypal")] + let paypal_config = if crate::company::grants_paypal_explicit( + &self.manifest.tools.allow, + ) { + crate::harness::paypal::TenantPaypal::resolve(&secrets, &id) + .await + .unwrap_or_else(|err| { + tracing::warn!( + company = %id, + "[paypal] could not read the billing credential at \ + boot; wiring no PayPal tools this turn: {err}" + ); + None + }) + } else { + None + }; + #[cfg(feature = "chargebee")] + let chargebee_config = if crate::company::grants_chargebee_explicit( + &self.manifest.tools.allow, + ) { + crate::harness::chargebee::TenantChargebee::resolve(&secrets, &id) + .await + .unwrap_or_else(|err| { + tracing::warn!( + company = %id, + "[chargebee] could not read the billing credential at \ + boot; wiring no Chargebee tools this turn: {err}" + ); + None + }) + } else { + None + }; let composio_config = if crate::company::grants_composio_explicit( &self.manifest.tools.allow, ) { @@ -2266,6 +2320,10 @@ impl RuntimeBuilder { // resolved above (token from the secret store, // never an env/platform key). `None` fails closed. composio: composio_config, + #[cfg(feature = "chargebee")] + chargebee: chargebee_config, + #[cfg(feature = "paypal")] + paypal: paypal_config, steer, run_supervisor: supervisor, // Issue #170: the ports an `output` node's diff --git a/src/server/hooks_chargebee.rs b/src/server/hooks_chargebee.rs new file mode 100644 index 000000000..e12374e07 --- /dev/null +++ b/src/server/hooks_chargebee.rs @@ -0,0 +1,722 @@ +//! The inbound Chargebee webhook: `POST /hooks/{company}/chargebee` (issue #788). +//! +//! Chargebee posts here when a payment succeeds or fails. A verified delivery +//! raises a [`CompanyEvent::WebhookReceived`] on the `chargebee` channel, which +//! drives one cycle — so the operator hears "Alan paid the $100 invoice" in +//! chat **without having asked**. That push is the whole point of this route. +//! +//! # Why this does not persist invoice state +//! +//! Issue #788's TC-03 describes the webhook updating stored state that the agent +//! then reads to answer "has Alan paid?". It deliberately does not do that. +//! `chargebee_get_invoice` already answers that question **live from +//! Chargebee**, and does it strictly better: stored state goes stale the moment +//! a delivery is dropped, retried, or replayed out of order, and then the agent +//! confidently reports a payment status that Chargebee disagrees with. A cache +//! that can silently diverge from the system of record is worse than no cache +//! when the subject is money. +//! +//! So the split is: **pull** stays live (`chargebee_get_invoice`), and this +//! route owns **push** — the thing a live read genuinely cannot do. +//! +//! # Verification comes before parsing +//! +//! Chargebee protects a webhook URL with HTTP Basic auth, configured beside the +//! URL in its dashboard. The handler compares that header, constant-time, +//! against the company's stored secret **before it parses anything**: an +//! unverifiable POST is dropped with `401` and never becomes an event. Same +//! order, and the same reason, as the Telegram hook next door. +//! +//! # The event names are Chargebee's, not the issue's +//! +//! #788 calls them `invoice_paid` and `invoice_payment_failed`. Chargebee has no +//! such events — the real ones are `payment_succeeded` and `payment_failed` +//! (plus `invoice_generated`). The names here are the ones an operator will +//! actually find in the Chargebee dashboard's webhook configuration. + +use std::sync::Arc; + +use axum::body::Bytes; +use axum::extract::Path; +use axum::http::{HeaderMap, StatusCode}; +use axum::response::{IntoResponse, Response}; +use axum::routing::post; +use axum::{Json, Router}; +use serde_json::{Value, json}; + +use crate::AppState; +use crate::company::runtime::CompanyRuntime; +use crate::ports::types::CompanyEvent; +use crate::server::ops::resolve; + +pub use crate::company::billing::WEBHOOK_SECRET_KEY; + +/// The channel a verified delivery is raised on. +pub const CHANNEL: &str = "chargebee"; + +/// The Chargebee events this route acts on. +/// +/// An unlisted event is acknowledged and ignored rather than refused: Chargebee +/// sends whatever the dashboard subscribes to, an operator will over-subscribe, +/// and answering non-2xx would make Chargebee retry — then disable the endpoint +/// — over an event we simply had no interest in. +const ACTED_ON: &[&str] = &["payment_succeeded", "payment_failed", "invoice_generated"]; + +/// The most body this route will buffer, well above any real Chargebee event. +/// +/// The second of two bounds, and the weaker one. [`VerifiedDelivery`] means an +/// unauthenticated caller's body is never read at all; this caps what a caller +/// who DID authenticate can make the host allocate. A Chargebee event is a few +/// KiB — an invoice with a long line-item list is the largest realistic case and +/// nowhere near this — so the 2 MiB axum defaults to is simply more room than +/// the endpoint has any use for. +const MAX_EVENT_BYTES: usize = 256 * 1024; + +/// Builds the Chargebee webhook route fragment. +pub fn router() -> Router { + Router::new().route( + "/hooks/{company}/chargebee", + post(chargebee_hook).layer(axum::extract::DefaultBodyLimit::max(MAX_EVENT_BYTES)), + ) +} + +/// A `401` drop for an unverifiable delivery. +fn unauthorized() -> Response { + ( + StatusCode::UNAUTHORIZED, + Json(json!({ "error": "invalid webhook credentials", "code": "unauthorized" })), + ) + .into_response() +} + +/// Length-checked, branch-independent byte comparison. +fn constant_time_eq(a: &[u8], b: &[u8]) -> bool { + if a.len() != b.len() { + return false; + } + let mut diff = 0u8; + for (x, y) in a.iter().zip(b.iter()) { + diff |= x ^ y; + } + diff == 0 +} + +/// Decodes an `Authorization: Basic ` header into `user:pass`. +/// +/// Hand-rolled because `base64` is behind the `mcp` feature and this route ships +/// in every build. Decoding is 20 lines; taking a feature dependency for it +/// would make an always-on route conditional on an unrelated one. +fn decode_basic(header: &str) -> Option { + let encoded = header.strip_prefix("Basic ")?.trim(); + const ALPHABET: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + + // Structure first. An earlier version decoded until it met a `=` or an + // unknown byte, so `QQ` and `QQ=garbage` both yielded a prefix that then + // went into a credential comparison — a decoder that accepts more than it + // should is a poor thing to put in front of an auth check, even when the + // comparison itself would fail. + let bytes = encoded.as_bytes(); + if bytes.is_empty() || bytes.len() % 4 != 0 { + return None; + } + let padding = bytes.iter().rev().take_while(|b| **b == b'=').count(); + if padding > 2 { + return None; + } + let body = &bytes[..bytes.len() - padding]; + if body.iter().any(|b| !ALPHABET.contains(b)) { + return None; + } + + let mut bits: u32 = 0; + let mut nbits = 0; + let mut out: Vec = Vec::new(); + for byte in body { + let value = ALPHABET.iter().position(|c| c == byte)? as u32; + bits = (bits << 6) | value; + nbits += 6; + if nbits >= 8 { + nbits -= 8; + out.push((bits >> nbits) as u8); + } + } + // Leftover bits must be zero in canonical base64; anything else means the + // input was not produced by an encoder. + if nbits > 0 && (bits & ((1 << nbits) - 1)) != 0 { + return None; + } + String::from_utf8(out).ok() +} + +/// A delivery whose credential has already been verified. +/// +/// This is an **extractor**, not a check inside the handler, and the difference +/// is the point: axum runs every `FromRequestParts` extractor before the one +/// `FromRequest` extractor that consumes the body, so an unverifiable POST is +/// rejected while its body is still on the socket. With the check inside the +/// handler, `Bytes` had already buffered whatever an unauthenticated caller +/// chose to send — bounded by the route's `DefaultBodyLimit`, but bounded is +/// not the same as never read. +/// +/// Carrying the runtime in the type also means the handler cannot forget: there +/// is no path to the body that does not go through a verified credential. +struct VerifiedDelivery(Arc); + +impl axum::extract::FromRequestParts for VerifiedDelivery { + type Rejection = Response; + + async fn from_request_parts( + parts: &mut axum::http::request::Parts, + state: &AppState, + ) -> std::result::Result { + let Path(company) = Path::::from_request_parts(parts, state) + .await + .map_err(IntoResponse::into_response)?; + let runtime = resolve(state, &company).map_err(IntoResponse::into_response)?; + verify(&runtime, &parts.headers).await?; + Ok(Self(runtime)) + } +} + +/// Compares the delivery's HTTP Basic credential against the company's stored +/// one, in constant time. +/// +/// A stored credential must exist to verify against. An empty stored value +/// counts as "not configured" — reject rather than accept anything. +async fn verify( + runtime: &Arc, + headers: &HeaderMap, +) -> std::result::Result<(), Response> { + let expected = match runtime + .secrets() + .get(runtime.id(), WEBHOOK_SECRET_KEY) + .await + { + Ok(Some(secret)) if !secret.expose().is_empty() => secret.expose().to_string(), + Ok(_) => return Err(unauthorized()), + Err(err) => return Err(crate::server::error::ApiError(err).into_response()), + }; + + let Some(provided) = headers + .get(axum::http::header::AUTHORIZATION) + .and_then(|v| v.to_str().ok()) + .and_then(decode_basic) + else { + return Err(unauthorized()); + }; + if !constant_time_eq(provided.as_bytes(), expected.as_bytes()) { + return Err(unauthorized()); + } + Ok(()) +} + +/// `POST /hooks/{company}/chargebee`. +/// +/// `VerifiedDelivery` comes first deliberately — it is the extractor that +/// authenticates, and `raw` is only read once it has succeeded. +async fn chargebee_hook(VerifiedDelivery(runtime): VerifiedDelivery, raw: Bytes) -> Response { + handle(runtime, &raw).await +} + +/// Raises one cycle for an event worth telling the operator about. +/// +/// The credential is already verified — see [`VerifiedDelivery`]. +async fn handle(runtime: Arc, raw: &[u8]) -> Response { + let Ok(event) = serde_json::from_slice::(raw) else { + // Malformed body from a caller that DID authenticate: accept it so + // Chargebee stops retrying, and say so in the log rather than silently. + tracing::warn!(company = %runtime.id().as_ref(), "[chargebee] webhook body was not JSON"); + return ( + StatusCode::OK, + Json(json!({"ok": true, "ignored": "unparseable"})), + ) + .into_response(); + }; + + let event_type = event + .get("event_type") + .and_then(Value::as_str) + .unwrap_or_default() + .to_string(); + if !ACTED_ON.contains(&event_type.as_str()) { + return ( + StatusCode::OK, + Json(json!({"ok": true, "ignored": event_type})), + ) + .into_response(); + } + + let summary = summarize(&event_type, &event); + tracing::info!(company = %runtime.id().as_ref(), %event_type, "[chargebee] webhook"); + + // Drive one cycle so the company *says* something. A paused or archived + // company acknowledges without running — Chargebee must not retry because + // the operator happened to have the company stopped. + if runtime.ensure_running().await.is_ok() { + let event = CompanyEvent::WebhookReceived { + channel: CHANNEL.to_string(), + // The summary, not the raw event: `body` reaches the brain, and a + // whole Chargebee payload spends a great deal of context to say + // "Alan paid". The original is still in the log line above. + body: json!({"event_type": event_type, "summary": summary}), + }; + if let Err(err) = runtime.run_cycle(vec![event]).await { + tracing::warn!(company = %runtime.id(), "chargebee cycle failed: {err}"); + } + } + + (StatusCode::OK, Json(json!({"ok": true}))).into_response() +} + +/// Renders the event as the sentence the agent is asked to relay. +/// +/// A summary rather than the raw payload: a Chargebee event carries the whole +/// invoice, customer, transaction and card objects, and handing that to a model +/// spends a large amount of context to say "Alan paid". Amounts stay in minor +/// units with the currency beside them — this text reaches a model, and a bare +/// `10000` with no unit is exactly how a $100 payment gets reported as $10,000. +fn summarize(event_type: &str, event: &Value) -> String { + let content = event.get("content").cloned().unwrap_or(Value::Null); + let invoice = content.get("invoice"); + let field = |obj: Option<&Value>, key: &str| -> Option { + obj?.get(key).and_then(Value::as_str).map(str::to_string) + }; + let id = field(invoice, "id").unwrap_or_else(|| "(unknown)".to_string()); + let currency = field(invoice, "currency_code").unwrap_or_default(); + let total = invoice + .and_then(|i| i.get("total")) + .and_then(Value::as_i64) + .map(|t| format!("{t} {currency} (minor units)")) + .unwrap_or_else(|| "an unknown amount".to_string()); + // The customer id, never their email. This string is persisted in the + // company journal and replayed into model prompts, so a counterparty's + // address would outlive the notification it was needed for. The id is + // sufficient to look them up in Chargebee. + let who = field(content.get("customer"), "id").unwrap_or_else(|| "the customer".to_string()); + + match event_type { + "payment_succeeded" => format!( + "Chargebee: invoice {id} for {total} was PAID by {who}. Tell the operator, briefly." + ), + "payment_failed" => format!( + "Chargebee: a payment FAILED for invoice {id} ({total}) from {who}. Tell the operator, \ + briefly, and say the invoice is still outstanding." + ), + _ => format!("Chargebee: invoice {id} for {total} was generated for {who}."), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // --- End-to-end, through the real router ------------------------------ + // + // The unit tests below cover `decode_basic` and `summarize`. These drive the + // whole route, because the thing worth protecting is not either function on + // its own — it is that an unverifiable POST cannot reach the parser, the + // event filter, or a company cycle. This is the one surface here that + // accepts input from outside the host. + + /// Every event the company's brain was actually driven with. + /// + /// The route answers `200` whether or not it raised anything, so the HTTP + /// response cannot distinguish a working webhook from a handler that + /// acknowledges and does nothing. This is the seam that can: it sits where + /// the cycle actually lands. + type Delivered = Arc>>; + + /// A brain that records what it was asked to run, then behaves as the + /// default one does. + /// + /// Delegating to [`EchoBrain`](crate::brain::EchoBrain) rather than + /// returning a hand-built `CycleResult` keeps the cycle on the path it takes + /// in these tests already — the recorder observes, it does not substitute. + struct RecordingBrain(Delivered); + + #[async_trait::async_trait] + impl crate::ports::brain::Brain for RecordingBrain { + async fn run_cycle( + &self, + req: crate::ports::types::CycleRequest, + host: &dyn crate::ports::brain::CycleHost, + ) -> crate::Result { + self.0 + .lock() + .expect("lock") + .extend(req.events.iter().cloned()); + crate::brain::EchoBrain::new().run_cycle(req, host).await + } + } + + /// A host with one company, and the webhook credential `credential` stored + /// when it is `Some`. + async fn state_with( + home: &std::path::Path, + credential: Option<&str>, + ) -> (AppState, Arc, Delivered) { + use crate::ports::{CompanyStore, types::CompanyRecord}; + use crate::store::FsCompanyStore; + + let id = crate::ports::types::CompanyId::new("acme"); + let manifest: crate::company::CompanyManifest = toml::from_str( + "[company]\nname = \"Acme\"\n[[agent]]\nid = \"ceo\"\nrole = \"Chief\"\n[policy]\nmode = \"full\"\n", + ) + .expect("manifest"); + FsCompanyStore::new(home.to_path_buf()) + .save(&CompanyRecord { + id: id.clone(), + manifest: manifest.clone(), + ledger: Vec::new(), + lifecycle: "running".to_string(), + overlay_agents: Vec::new(), + overlay_desk_members: Vec::new(), + overlay_desk_order: Vec::new(), + overlay_desk_tools: Default::default(), + overlay_desks: Vec::new(), + overlay_workflows: Vec::new(), + overlay_budgets: Vec::new(), + overlay_policy: None, + disabled_workflows: Vec::new(), + template_provenance: None, + }) + .await + .expect("save company"); + + let delivered: Delivered = Arc::new(std::sync::Mutex::new(Vec::new())); + let runtime = Arc::new( + crate::runtime::RuntimeBuilder::new(home.to_path_buf(), manifest) + .with_id(id.clone()) + .with_brain(Arc::new(RecordingBrain(delivered.clone()))) + .build() + .await + .expect("runtime"), + ); + if let Some(credential) = credential { + runtime + .secrets() + .set( + runtime.id(), + WEBHOOK_SECRET_KEY, + crate::ports::types::SecretValue(credential.to_string()), + ) + .await + .expect("store credential"); + } + let state = AppState::new(crate::AppConfig::default()); + state.registry().insert(id, runtime.clone()); + (state, runtime, delivered) + } + + /// Posts `body` to the route, with `auth` verbatim as the header value. + async fn post_event(state: &AppState, auth: Option<&str>, body: Value) -> (StatusCode, Value) { + use axum::body::{Body, to_bytes}; + use tower::ServiceExt; + + let mut request = axum::http::Request::builder() + .method("POST") + .uri("/hooks/acme/chargebee") + .header("content-type", "application/json"); + if let Some(auth) = auth { + request = request.header("authorization", auth); + } + let request = request.body(Body::from(body.to_string())).expect("request"); + let response = crate::server::router(state.clone()) + .oneshot(request) + .await + .expect("routed"); + let status = response.status(); + let bytes = to_bytes(response.into_body(), usize::MAX) + .await + .expect("body"); + ( + status, + serde_json::from_slice(&bytes).unwrap_or(Value::Null), + ) + } + + fn paid_event() -> Value { + json!({ + "event_type": "payment_succeeded", + "content": { + "invoice": {"id": "inv_1", "currency_code": "USD", "total": 10000}, + "customer": {"id": "cus_1", "email": "alan@tinyhumans.ai"} + } + }) + } + + #[tokio::test] + async fn an_unverifiable_delivery_is_refused_and_never_becomes_an_event() { + let home = tempfile::tempdir().expect("tempdir"); + // base64("cbuser:cbpass") + let (state, _runtime, delivered) = state_with(home.path(), Some("cbuser:cbpass")).await; + + for (label, auth) in [ + ("no header at all", None), + ("a wrong password", Some("Basic Y2J1c2VyOndyb25n")), + ("a bearer token", Some("Bearer Y2J1c2VyOmNicGFzcw==")), + ("a malformed encoding", Some("Basic QQ=garbage")), + ] { + let (status, body) = post_event(&state, auth, paid_event()).await; + assert_eq!(status, StatusCode::UNAUTHORIZED, "{label}: {body}"); + assert_eq!(body["code"], "unauthorized", "{label}"); + } + + // The `401` is the visible half. The half that matters is that none of + // those four reached a cycle: an unverifiable POST must not be able to + // drive the company at all. + assert!( + delivered.lock().expect("lock").is_empty(), + "an unverifiable delivery drove a cycle: {:?}", + delivered.lock().expect("lock"), + ); + } + + #[tokio::test] + async fn a_company_with_no_stored_credential_accepts_nothing() { + // Fail closed: an unconfigured webhook must not be an open endpoint. + // Without the stored-secret check, "no credential" would be the one + // state in which any caller could drive a company cycle. + let home = tempfile::tempdir().expect("tempdir"); + let (state, runtime, _delivered) = state_with(home.path(), None).await; + + let (status, _) = + post_event(&state, Some("Basic Y2J1c2VyOmNicGFzcw=="), paid_event()).await; + assert_eq!(status, StatusCode::UNAUTHORIZED, "nothing stored"); + + // And an EMPTY stored value counts as unconfigured, which is how the + // console clears a credential — the secret port has no delete. + runtime + .secrets() + .set( + runtime.id(), + WEBHOOK_SECRET_KEY, + crate::ports::types::SecretValue(String::new()), + ) + .await + .expect("clear"); + let (status, _) = + post_event(&state, Some("Basic Y2J1c2VyOmNicGFzcw=="), paid_event()).await; + assert_eq!(status, StatusCode::UNAUTHORIZED, "cleared to empty"); + } + + #[tokio::test] + async fn a_verified_delivery_is_accepted_and_actually_raises_the_event() { + // The `200` proves almost nothing on its own: this route answers `200` + // for an ignored event, an unparseable body, and a paused company too. A + // handler that dropped the `CompanyEvent::WebhookReceived` construction + // or the `run_cycle` call entirely would still satisfy it — and the push + // is the ONLY thing this route does that a live read cannot, so a + // regression there silently removes the whole point of the endpoint. + // + // So the assertion is on what reached the brain. + let home = tempfile::tempdir().expect("tempdir"); + let (state, _runtime, delivered) = state_with(home.path(), Some("cbuser:cbpass")).await; + + let (status, body) = + post_event(&state, Some("Basic Y2J1c2VyOmNicGFzcw=="), paid_event()).await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(body["ok"], true); + assert_eq!( + body["ignored"], + Value::Null, + "an acted-on event is not ignored" + ); + + let events = delivered.lock().expect("lock").clone(); + let raised = events + .iter() + .find_map(|event| match event { + CompanyEvent::WebhookReceived { channel, body } if channel == CHANNEL => { + Some(body.clone()) + } + _ => None, + }) + .unwrap_or_else(|| { + panic!("no WebhookReceived on the `{CHANNEL}` channel reached a cycle: {events:?}") + }); + + // And it carries the summary the brain is meant to relay, not the raw + // Chargebee payload — the projection is part of the contract, since the + // whole event would spend a great deal of context to say "Alan paid". + assert_eq!(raised["event_type"], "payment_succeeded", "{raised}"); + let summary = raised["summary"].as_str().unwrap_or_default(); + assert!(summary.contains("inv_1"), "{summary}"); + assert!(summary.contains("PAID"), "{summary}"); + // The customer id, never their email — this body is persisted in the + // journal and replayed into model prompts. + assert!(summary.contains("cus_1"), "{summary}"); + assert!(!raised.to_string().contains("alan@"), "{raised}"); + } + + #[tokio::test] + async fn a_verified_but_unsubscribed_event_never_reaches_a_cycle() { + // The `ignored` field in the response says the route decided to skip it. + // This says it actually did: an over-subscribed dashboard must not wake + // the company on every subscription change it happens to send. + let home = tempfile::tempdir().expect("tempdir"); + let (state, _runtime, delivered) = state_with(home.path(), Some("cbuser:cbpass")).await; + + post_event( + &state, + Some("Basic Y2J1c2VyOmNicGFzcw=="), + json!({"event_type": "subscription_created", "content": {}}), + ) + .await; + assert!( + delivered.lock().expect("lock").is_empty(), + "an unsubscribed event drove a cycle: {:?}", + delivered.lock().expect("lock"), + ); + } + + #[tokio::test] + async fn a_verified_but_unsubscribed_event_is_acknowledged_and_ignored() { + // 2xx on purpose: answering non-2xx would make Chargebee retry, then + // disable the endpoint, over an event we simply had no interest in. + let home = tempfile::tempdir().expect("tempdir"); + let (state, _runtime, _delivered) = state_with(home.path(), Some("cbuser:cbpass")).await; + + let (status, body) = post_event( + &state, + Some("Basic Y2J1c2VyOmNicGFzcw=="), + json!({"event_type": "subscription_created", "content": {}}), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(body["ignored"], "subscription_created"); + } + + #[test] + fn basic_auth_decodes_to_the_user_pass_pair() { + // base64("cbuser:cbpass") + assert_eq!( + decode_basic("Basic Y2J1c2VyOmNicGFzcw==").as_deref(), + Some("cbuser:cbpass") + ); + // Anything that is not Basic is not ours to interpret. + assert_eq!(decode_basic("Bearer abc"), None); + assert_eq!(decode_basic("Basic !!!not base64!!!"), None); + + // Malformed input must be REFUSED, not decoded to a prefix that then + // reaches a credential comparison. + assert_eq!(decode_basic("Basic QQ"), None, "length not a multiple of 4"); + assert_eq!(decode_basic("Basic QQ=garbage"), None, "data after padding"); + assert_eq!(decode_basic("Basic ===="), None, "padding only"); + assert_eq!(decode_basic("Basic "), None, "empty"); + assert_eq!(decode_basic("Basic QUJD!"), None, "alphabet violation"); + // Canonical padding still works. + assert_eq!(decode_basic("Basic QUJD").as_deref(), Some("ABC")); + assert_eq!(decode_basic("Basic QUI=").as_deref(), Some("AB")); + } + + #[test] + fn a_long_credential_decodes_exactly() { + // The accumulator is never masked, so it visibly "overflows" a u32 after + // a handful of characters. That is fine and this pins why: `<<` in Rust + // only panics when the SHIFT AMOUNT reaches the width — a constant 6 + // here — and bits pushed off the top are discarded by definition, while + // the decoder only ever reads the low `nbits` (at most 6) it just wrote. + // A wider type or a mask would change nothing. + fn encode(input: &[u8]) -> String { + const A: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + let mut out = String::new(); + for chunk in input.chunks(3) { + let b = [ + chunk[0], + *chunk.get(1).unwrap_or(&0), + *chunk.get(2).unwrap_or(&0), + ]; + let n = ((b[0] as u32) << 16) | ((b[1] as u32) << 8) | b[2] as u32; + out.push(A[(n >> 18 & 63) as usize] as char); + out.push(A[(n >> 12 & 63) as usize] as char); + out.push(if chunk.len() > 1 { + A[(n >> 6 & 63) as usize] as char + } else { + '=' + }); + out.push(if chunk.len() > 2 { + A[(n & 63) as usize] as char + } else { + '=' + }); + } + out + } + + for len in [1usize, 2, 3, 300, 30_000] { + let credential = format!("cbuser:{}", "x".repeat(len)); + let decoded = decode_basic(&format!("Basic {}", encode(credential.as_bytes()))); + assert_eq!( + decoded.as_deref(), + Some(credential.as_str()), + "a {len}-byte password must round-trip exactly" + ); + } + } + + #[test] + fn constant_time_eq_still_compares_correctly() { + assert!(constant_time_eq(b"secret", b"secret")); + assert!(!constant_time_eq(b"secret", b"secreT")); + // A length difference must not be reported as equal. + assert!(!constant_time_eq(b"secret", b"secretx")); + } + + #[test] + fn only_the_three_billing_events_are_acted_on() { + // Over-subscribing in the Chargebee dashboard is the normal case; an + // unlisted event must be ignorable, not a reason to retry. + assert!(ACTED_ON.contains(&"payment_succeeded")); + assert!(ACTED_ON.contains(&"payment_failed")); + assert!(!ACTED_ON.contains(&"subscription_created")); + // The names #788 uses do not exist in Chargebee; if these ever start + // matching, the issue's names were adopted and this test should say so. + assert!(!ACTED_ON.contains(&"invoice_paid")); + } + + #[test] + fn a_paid_summary_names_the_invoice_amount_and_payer() { + let event = json!({ + "event_type": "payment_succeeded", + "content": { + "invoice": {"id": "inv_42", "total": 10000, "currency_code": "USD"}, + "customer": {"id": "cus_7", "email": "alan@tinyhumans.ai"} + } + }); + let text = summarize("payment_succeeded", &event); + assert!(text.contains("inv_42"), "{text}"); + assert!(text.contains("PAID"), "{text}"); + // The id identifies them; the EMAIL must not travel. This string is + // persisted in the journal and replayed into model prompts, so a + // counterparty's address would outlive the notification. + assert!(text.contains("cus_7"), "{text}"); + assert!(!text.contains("alan@tinyhumans.ai"), "email leaked: {text}"); + // The unit must travel with the number or $100 gets reported as $10,000. + assert!(text.contains("minor units"), "{text}"); + } + + #[test] + fn a_failed_summary_says_the_invoice_is_still_outstanding() { + let event = json!({ + "event_type": "payment_failed", + "content": {"invoice": {"id": "inv_9", "total": 500, "currency_code": "USD"}} + }); + let text = summarize("payment_failed", &event); + assert!(text.contains("FAILED"), "{text}"); + assert!(text.contains("outstanding"), "{text}"); + // No customer object in the payload is normal; it must not panic or + // render an empty gap where a person should be. + assert!(text.contains("the customer"), "{text}"); + } + + #[test] + fn a_summary_survives_a_payload_with_no_content() { + let text = summarize( + "payment_succeeded", + &json!({"event_type": "payment_succeeded"}), + ); + assert!(text.contains("(unknown)"), "{text}"); + assert!(text.contains("an unknown amount"), "{text}"); + } +} diff --git a/src/server/mod.rs b/src/server/mod.rs index 4ad8b2c4e..0dd16bd9a 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -15,6 +15,7 @@ mod error; pub mod feedback; pub mod graphql; pub mod hooks; +pub mod hooks_chargebee; pub mod hub_identity; // Console MCP OAuth callback (issue #90): the unauthenticated browser-redirect // landing route. Gated on `mcp` (it needs the OAuth token-exchange path). diff --git a/src/server/operator.rs b/src/server/operator.rs index 83306cdf2..587726be5 100644 --- a/src/server/operator.rs +++ b/src/server/operator.rs @@ -2660,6 +2660,10 @@ mod test { plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), delivery: None, search: None, diff --git a/src/server/ops/billing.rs b/src/server/ops/billing.rs new file mode 100644 index 000000000..d02eb6fc5 --- /dev/null +++ b/src/server/ops/billing.rs @@ -0,0 +1,1029 @@ +//! The Chargebee billing configuration write-plane (issue #788, UI in #527): +//! store the API key, the site identifier and the webhook credential — all +//! **write-only** — and surface the webhook URL an operator pastes into +//! Chargebee. +//! +//! `GET …/billing/chargebee` returns only [`BillingStatus`], which carries +//! booleans and the site slug. The API key and the webhook credential are never +//! serialized into any response, by construction: they live in +//! [`SecretStore`](crate::ports::SecretStore) and this module reads them back +//! only to *use* them, never to echo them. +//! +//! # Why the site identifier is a secret too +//! +//! It is not confidential, and it *is* returned by `GET` — a settings form has +//! to show what it is configured against, and "Connected ✓" beside the wrong +//! site is exactly the confusion this avoids. It shares the secret store with +//! the key only because the pair is meaningless apart: the tools need both or +//! neither, so keeping them in one place makes "half configured" impossible to +//! express by accident. +//! +//! # Three things can each be missing, and they fail differently +//! +//! [`BillingStatus`] reports them separately rather than as one "connected" +//! flag, because the remedies differ: no key or site means the agent has no +//! billing tools at all; no webhook credential means the tools work but nobody +//! is told when a customer pays; and a missing `chargebee` grant means both are +//! configured and still nothing reaches an agent. A single boolean would send an +//! operator looking in the wrong place for two of those three. + +use axum::extract::State; +use axum::routing::{delete, get}; +use axum::{Json, Router}; +use serde::{Deserialize, Serialize}; + +use crate::AppState; +use crate::company::billing::{API_KEY_SECRET, SITE_SECRET, WEBHOOK_SECRET_KEY}; +use crate::company::paypal::{ + CLIENT_ID_SECRET, CLIENT_SECRET_SECRET, ENVIRONMENT_SECRET, PaypalEnvironment, +}; +use crate::company::runtime::CompanyRuntime; +use crate::ports::types::{CompanyId, SecretValue}; +use crate::server::error::ApiError; +use crate::server::ops::scope::{AdminScopedCompany, ScopedCompany, scoped}; + +/// The non-secret view of a company's Chargebee configuration. +#[derive(Debug, Serialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct BillingStatus { + /// Whether an API key is stored and non-empty. Never the key itself. + pub api_key_configured: bool, + /// The Chargebee site slug, when set — shown so a settings form can say + /// *which* site it is connected to rather than only that it is. + pub site: Option, + /// Whether a webhook credential is stored, i.e. whether a delivery from + /// Chargebee could be verified at all. + pub webhook_configured: bool, + /// The URL to paste into Chargebee's webhook settings. `None` on a host with + /// no publicly reachable base URL — Chargebee cannot deliver to a loopback + /// address, and showing one would send an operator to configure a webhook + /// that silently never arrives (the shape of issue #203). + pub webhook_url: Option, + /// Whether this company's manifest **explicitly** grants `chargebee`. + /// Both credentials can be present and still wire no tools without it. + pub granted: bool, + /// Whether the `chargebee` feature is compiled into this build at all. + pub in_build: bool, +} + +/// The non-secret view of a company's PayPal connection (issue #789). +#[derive(Debug, Serialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct PaypalStatus { + /// Whether a client id is stored. Never the id itself — it is half a + /// credential, and there is no reason to render it back. + pub client_id_configured: bool, + /// Whether a client secret is stored. + pub client_secret_configured: bool, + /// `sandbox` or `live`. Shown, because "Connected" against the wrong world + /// is the confusion this exists to avoid. + pub environment: String, + /// Whether this company's manifest explicitly grants `paypal`. + pub granted: bool, + /// Whether the `paypal` feature is compiled into this build. + pub in_build: bool, +} + +/// Builds the billing configuration routes. +pub fn router() -> Router { + scoped("/billing/chargebee", get(get_billing).put(put_billing)) + .merge(scoped("/billing/chargebee/key", delete(delete_billing))) + .merge(scoped("/billing/paypal", get(get_paypal).put(put_paypal))) + .merge(scoped("/billing/paypal/key", delete(delete_paypal))) +} + +/// Assembles the non-secret PayPal status. +async fn paypal_status_of(runtime: &CompanyRuntime) -> Result { + let granted = runtime + .store() + .load(runtime.id()) + .await + .ok() + .flatten() + .map(|record| crate::company::grants_paypal_explicit(&record.manifest.tools.allow)) + .unwrap_or(false); + let environment = read(runtime, ENVIRONMENT_SECRET) + .await? + .map(|raw| PaypalEnvironment::parse(&raw)) + .unwrap_or_default(); + Ok(PaypalStatus { + client_id_configured: read(runtime, CLIENT_ID_SECRET).await?.is_some(), + client_secret_configured: read(runtime, CLIENT_SECRET_SECRET).await?.is_some(), + environment: environment.as_str().to_string(), + granted, + in_build: cfg!(feature = "paypal"), + }) +} + +/// `GET …/billing/paypal` — non-secret status only. +async fn get_paypal(company: ScopedCompany) -> Result, ApiError> { + Ok(Json(paypal_status_of(&company.runtime).await?)) +} + +/// The write-only PayPal config body. +#[derive(Debug, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +struct PaypalConfigBody { + /// REST app client id (write-only). Omit to leave unchanged. + #[serde(default)] + client_id: Option, + /// REST app secret (write-only). Omit to leave unchanged. + #[serde(default)] + client_secret: Option, + /// `sandbox` or `live`. Anything unrecognised stores `sandbox`. + #[serde(default)] + environment: Option, +} + +/// `PUT …/billing/paypal` — store any supplied credentials, return status. +/// +/// Admin-only, like its Chargebee sibling: pointing a company at a different +/// PayPal account changes whose wallet its agents can read. +async fn put_paypal( + company: AdminScopedCompany, + Json(body): Json, +) -> Result, ApiError> { + let runtime = &company.runtime; + let supplied = |value: Option<&str>| { + value + .map(str::trim) + .filter(|v| !v.is_empty()) + .map(str::to_string) + }; + + // Collected and then applied together — a client id stored without its + // secret is exactly the half-configured state `write_all` exists to prevent. + let mut writes: Vec<(&str, String)> = Vec::new(); + if let Some(client_id) = supplied(body.client_id.as_deref()) { + writes.push((CLIENT_ID_SECRET, client_id)); + } + if let Some(client_secret) = supplied(body.client_secret.as_deref()) { + writes.push((CLIENT_SECRET_SECRET, client_secret)); + } + if let Some(raw) = body.environment.as_deref() { + // Normalised through the same parser the client uses, so an unrecognised + // value is stored as `sandbox` rather than kept verbatim and re-parsed + // differently somewhere else later. + writes.push(( + ENVIRONMENT_SECRET, + PaypalEnvironment::parse(raw).as_str().to_string(), + )); + } + write_all(runtime, &writes).await?; + + Ok(Json(paypal_status_of(runtime).await?)) +} + +/// `DELETE …/billing/paypal/key` — clear the stored PayPal credentials. +/// +/// The environment is cleared too, so a re-connect starts from the safe default +/// rather than silently inheriting `live` from a previous account. +async fn delete_paypal(company: AdminScopedCompany) -> Result, ApiError> { + let runtime = &company.runtime; + // Together, for the same reason as the write: a clear that dropped the + // client id and then failed would leave a secret with no id — still "half + // configured", and reported by `PaypalStatus` as such. + let cleared: Vec<(&str, String)> = [CLIENT_ID_SECRET, CLIENT_SECRET_SECRET, ENVIRONMENT_SECRET] + .into_iter() + .map(|key| (key, String::new())) + .collect(); + write_all(runtime, &cleared).await?; + Ok(Json(paypal_status_of(runtime).await?)) +} + +/// The webhook URL for `company`, or `None` when this host has no publicly +/// reachable base URL. +/// +/// Deliberately the same source as the telegram channel's — not the bind +/// address, which yields a `http://127.0.0.1:/…` URL that is +/// syntactically fine and undeliverable in practice. +fn webhook_url(state: &AppState, company: &CompanyId) -> Option { + let base = state.config().public_webhook_base_url()?; + Some(format!("{base}/hooks/{}/chargebee", company.as_ref())) +} + +/// Applies a batch of credential writes so a failure part-way through cannot +/// leave a company half configured. +/// +/// The module header says a half-configured company is "impossible to express by +/// accident". Sequential `set` calls, each with its own `?`, did not deliver +/// that: a store that accepted the API key and then failed on the webhook +/// credential returned an error to an operator whose key had nonetheless been +/// stored — and the pair is meaningless apart, which is the whole reason they +/// live in one place. +/// +/// [`SecretStore`](crate::ports::SecretStore) has neither a transaction nor a +/// delete; `set` is its entire write surface. So atomicity is built here: every +/// key's prior value is read first, and a failure restores the ones already +/// written before the original error is returned. A key with no prior value is +/// restored to the empty string, which is how this module already spells "unset" +/// (see [`delete_billing`]) and what every read site already treats as absent. +/// +/// **The rollback is best-effort, by necessity.** It is itself a sequence of +/// `set` calls against a store that has just failed one, so it can fail too. +/// What it cannot undo it logs at `error` with the key named, because an +/// operator told "save failed" who then finds a credential stored anyway has no +/// way to discover that on their own. +async fn write_all(runtime: &CompanyRuntime, writes: &[(&str, String)]) -> Result<(), ApiError> { + // Snapshot first. Reading after a partial write would capture the value this + // function itself just stored and roll back to it. + let mut prior: Vec<(&str, String)> = Vec::with_capacity(writes.len()); + for (key, _) in writes { + prior.push(( + key, + runtime + .secrets() + .get(runtime.id(), key) + .await? + .map(|value| value.expose().to_string()) + .unwrap_or_default(), + )); + } + + for (index, (key, value)) in writes.iter().enumerate() { + let Err(err) = runtime + .secrets() + .set(runtime.id(), key, SecretValue(value.clone())) + .await + else { + continue; + }; + for (done, before) in &prior[..index] { + if let Err(undo) = runtime + .secrets() + .set(runtime.id(), done, SecretValue(before.clone())) + .await + { + tracing::error!( + company = %runtime.id(), + key = done, + "[billing] a credential write failed and could not be rolled back; this \ + company is now half configured: {undo}" + ); + } + } + return Err(ApiError(err)); + } + Ok(()) +} + +/// Reads a stored secret, treating empty as absent. +async fn read(runtime: &CompanyRuntime, key: &str) -> Result, ApiError> { + Ok(runtime + .secrets() + .get(runtime.id(), key) + .await? + .map(|value| value.expose().to_string()) + .filter(|value| !value.trim().is_empty())) +} + +/// Assembles the non-secret status. +async fn status_of(state: &AppState, runtime: &CompanyRuntime) -> Result { + // The grant lives in the stored manifest, not on the runtime handle. A + // company that cannot be loaded reports `granted: false` rather than + // failing the whole status: the operator still needs to see what IS + // configured, and a settings page that 500s tells them nothing. + let granted = runtime + .store() + .load(runtime.id()) + .await + .ok() + .flatten() + .map(|record| crate::company::grants_chargebee_explicit(&record.manifest.tools.allow)) + .unwrap_or(false); + Ok(BillingStatus { + api_key_configured: read(runtime, API_KEY_SECRET).await?.is_some(), + site: read(runtime, SITE_SECRET).await?, + webhook_configured: read(runtime, WEBHOOK_SECRET_KEY).await?.is_some(), + webhook_url: webhook_url(state, runtime.id()), + granted, + in_build: cfg!(feature = "chargebee"), + }) +} + +/// `GET …/billing/chargebee` — non-secret status only. +async fn get_billing( + company: ScopedCompany, + State(state): State, +) -> Result, ApiError> { + Ok(Json(status_of(&state, &company.runtime).await?)) +} + +/// The write-only config body. Every field is optional; only fields present and +/// non-empty are applied, so the site can be corrected without re-entering the +/// key. +#[derive(Debug, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +struct BillingConfigBody { + /// The Chargebee API key (write-only). Omit to leave it unchanged. + #[serde(default)] + api_key: Option, + /// The site identifier — the `acme-test` in `acme-test.chargebee.com`. + #[serde(default)] + site: Option, + /// The `username:password` pair Chargebee is configured to present on its + /// webhook deliveries (write-only). Omit to leave it unchanged. + #[serde(default)] + webhook_secret: Option, +} + +/// Normalises a site identifier an operator may paste in several shapes. +/// +/// `acme-test`, `acme-test.chargebee.com` and `https://acme-test.chargebee.com/` +/// all mean the same site, and all three are what somebody actually pastes out +/// of a browser address bar. Storing the second or third produces a base URL of +/// `https://acme-test.chargebee.com.chargebee.com/api/v2`, whose failure names +/// DNS rather than the typo. +fn normalize_site(raw: &str) -> String { + raw.trim() + .trim_start_matches("https://") + .trim_start_matches("http://") + .trim_end_matches('/') + .split('.') + .next() + .unwrap_or_default() + .to_string() +} + +/// `PUT …/billing/chargebee` — store any supplied credentials, return status. +/// +/// Requires authority over the company: this key can raise invoices against +/// real customers in the company's name, so pointing it at a different +/// Chargebee site is not an ordinary member's edit. +async fn put_billing( + company: AdminScopedCompany, + State(state): State, + Json(body): Json, +) -> Result, ApiError> { + let runtime = &company.runtime; + let supplied = |value: Option<&str>| { + value + .map(str::trim) + .filter(|v| !v.is_empty()) + .map(str::to_string) + }; + + // Collected and then applied together. Written one `?` at a time, a store + // that took the API key and then failed on the webhook credential left the + // key stored behind an error response — the half-configured state this + // module's header claims cannot be reached by accident. + let mut writes: Vec<(&str, String)> = Vec::new(); + if let Some(api_key) = supplied(body.api_key.as_deref()) { + writes.push((API_KEY_SECRET, api_key)); + } + if let Some(webhook_secret) = supplied(body.webhook_secret.as_deref()) { + writes.push((WEBHOOK_SECRET_KEY, webhook_secret)); + } + if let Some(site) = body + .site + .as_deref() + .map(normalize_site) + .filter(|s| !s.is_empty()) + { + writes.push((SITE_SECRET, site)); + } + write_all(runtime, &writes).await?; + + Ok(Json(status_of(&state, runtime).await?)) +} + +/// `DELETE …/billing/chargebee/key` — clear every stored credential. +/// +/// The [`SecretStore`](crate::ports::SecretStore) port has no delete, so a +/// cleared credential is stored as the empty string; every read site treats an +/// empty value as unset (the tools fail closed, the webhook rejects). +async fn delete_billing( + company: AdminScopedCompany, + State(state): State, +) -> Result, ApiError> { + let runtime = &company.runtime; + // Together: a clear that dropped the key and then failed on the webhook + // credential would report the integration as disconnected while leaving the + // webhook endpoint live — the failure `clearing_removes_the_webhook_secret_ + // too_not_just_the_key` guards against, arrived at by a different route. + let cleared: Vec<(&str, String)> = [API_KEY_SECRET, SITE_SECRET, WEBHOOK_SECRET_KEY] + .into_iter() + .map(|key| (key, String::new())) + .collect(); + write_all(runtime, &cleared).await?; + Ok(Json(status_of(&state, runtime).await?)) +} + +#[cfg(test)] +mod tests { + use super::*; + + // --- The routes, end to end ------------------------------------------- + // + // The unit tests below cover the helpers. These drive the real router, + // because the properties worth holding are route-level: that a credential + // goes in and never comes back out, that clearing clears ALL of it, and + // that a member cannot read or write another role's billing settings. + + use axum::body::{Body, to_bytes}; + use axum::http::{Request, StatusCode}; + use serde_json::{Value, json}; + use tower::ServiceExt; + + async fn state_with_company(home: &std::path::Path) -> AppState { + use crate::ports::CompanyStore; + use crate::ports::types::CompanyRecord; + + let id = CompanyId::new("acme"); + let manifest: crate::company::CompanyManifest = ::toml::from_str( + "[company]\nname = \"Acme\"\n[[agent]]\nid = \"ceo\"\nrole = \"Chief\"\n[policy]\nmode = \"full\"\n", + ) + .expect("manifest"); + crate::store::FsCompanyStore::new(home.to_path_buf()) + .save(&CompanyRecord { + id: id.clone(), + manifest: manifest.clone(), + ledger: Vec::new(), + lifecycle: "running".to_string(), + overlay_agents: Vec::new(), + overlay_desk_members: Vec::new(), + overlay_desk_order: Vec::new(), + overlay_desk_tools: Default::default(), + overlay_desks: Vec::new(), + overlay_workflows: Vec::new(), + overlay_budgets: Vec::new(), + overlay_policy: None, + disabled_workflows: Vec::new(), + template_provenance: None, + }) + .await + .expect("save"); + + let runtime = crate::runtime::RuntimeBuilder::new(home.to_path_buf(), manifest) + .with_id(id.clone()) + .build() + .await + .expect("runtime"); + let state = AppState::new(crate::AppConfig::default()); + state.registry().insert(id, std::sync::Arc::new(runtime)); + state + } + + async fn call( + state: &AppState, + method: &str, + uri: &str, + cookie: &str, + body: Option, + ) -> (StatusCode, Value) { + // `seed_admin` / `seed_session` hand back a ready `Cookie` header value — + // these routes authenticate a signed-in human, not a bearer token. + let request = Request::builder() + .method(method) + .uri(uri) + .header("cookie", cookie); + let request = match body { + Some(body) => request + .header("content-type", "application/json") + .body(Body::from(body.to_string())), + None => request.body(Body::empty()), + } + .expect("request"); + let response = crate::server::router(state.clone()) + .oneshot(request) + .await + .expect("routed"); + let status = response.status(); + let bytes = to_bytes(response.into_body(), usize::MAX) + .await + .expect("body"); + ( + status, + serde_json::from_slice(&bytes).unwrap_or(Value::Null), + ) + } + + #[tokio::test] + async fn a_saved_credential_is_reported_as_configured_and_never_returned() { + let home = ::tempfile::tempdir().expect("tempdir"); + let state = state_with_company(home.path()).await; + let admin = crate::server::test_support::seed_admin(&state, "acme").await; + + // Nothing stored yet. + let (status, before) = call( + &state, + "GET", + "/api/v1/companies/acme/billing/chargebee", + &admin, + None, + ) + .await; + assert_eq!(status, StatusCode::OK, "{before}"); + assert_eq!(before["apiKeyConfigured"], false); + assert_eq!(before["webhookConfigured"], false); + + let (status, saved) = call( + &state, + "PUT", + "/api/v1/companies/acme/billing/chargebee", + &admin, + Some(json!({ + "apiKey": "cb_live_supersecret", + "site": "https://acme-test.chargebee.com", + "webhookSecret": "cbuser:cbpass", + })), + ) + .await; + assert_eq!(status, StatusCode::OK, "{saved}"); + + // The whole contract of this surface: it reports WHETHER a credential + // is stored, never what it is. A response that echoed the key back + // would put it in the browser, the network log and any screen share. + let (_, after) = call( + &state, + "GET", + "/api/v1/companies/acme/billing/chargebee", + &admin, + None, + ) + .await; + assert_eq!(after["apiKeyConfigured"], true); + assert_eq!(after["webhookConfigured"], true); + // The site is the one NON-secret field, and comes back normalised. + assert_eq!(after["site"], "acme-test"); + for rendered in [saved.to_string(), after.to_string()] { + assert!(!rendered.contains("cb_live_supersecret"), "{rendered}"); + assert!(!rendered.contains("cbpass"), "{rendered}"); + } + } + + #[tokio::test] + async fn clearing_removes_the_webhook_secret_too_not_just_the_key() { + // The route is named `…/key`, which reads as if it clears only the API + // key — leaving a webhook credential behind would keep the endpoint + // live while the UI reported the integration as cleared. + let home = ::tempfile::tempdir().expect("tempdir"); + let state = state_with_company(home.path()).await; + let admin = crate::server::test_support::seed_admin(&state, "acme").await; + + call( + &state, + "PUT", + "/api/v1/companies/acme/billing/chargebee", + &admin, + Some(json!({ + "apiKey": "cb_key", + "site": "acme-test", + "webhookSecret": "cbuser:cbpass", + })), + ) + .await; + let (status, cleared) = call( + &state, + "DELETE", + "/api/v1/companies/acme/billing/chargebee/key", + &admin, + None, + ) + .await; + assert_eq!(status, StatusCode::OK, "{cleared}"); + assert_eq!(cleared["apiKeyConfigured"], false); + assert_eq!(cleared["webhookConfigured"], false, "{cleared}"); + assert_eq!( + cleared["site"], + Value::Null, + "the site is cleared as well: {cleared}" + ); + } + + #[tokio::test] + async fn paypal_clears_its_environment_so_a_reconnect_starts_at_sandbox() { + // Inheriting `live` from a previous account is the failure worth + // preventing here: the next connection would read real money. + let home = ::tempfile::tempdir().expect("tempdir"); + let state = state_with_company(home.path()).await; + let admin = crate::server::test_support::seed_admin(&state, "acme").await; + + call( + &state, + "PUT", + "/api/v1/companies/acme/billing/paypal", + &admin, + Some(json!({ + "clientId": "AY_id", + "clientSecret": "EL_secret", + "environment": "live", + })), + ) + .await; + let (_, live) = call( + &state, + "GET", + "/api/v1/companies/acme/billing/paypal", + &admin, + None, + ) + .await; + assert_eq!(live["environment"], "live"); + assert_eq!(live["clientSecretConfigured"], true); + assert!(!live.to_string().contains("EL_secret"), "{live}"); + + let (status, cleared) = call( + &state, + "DELETE", + "/api/v1/companies/acme/billing/paypal/key", + &admin, + None, + ) + .await; + assert_eq!(status, StatusCode::OK, "{cleared}"); + assert_eq!(cleared["clientIdConfigured"], false); + assert_eq!(cleared["clientSecretConfigured"], false); + assert_eq!(cleared["environment"], "sandbox", "{cleared}"); + } + + /// An in-memory secret store whose `set` refuses one nominated key. + /// + /// The failure worth simulating is a store that works, then stops working + /// mid-batch — a `set` that times out, a full disk, a dropped Mongo + /// connection. A store that fails everything would never get far enough to + /// leave the half-configured state. + #[derive(Default)] + struct FailsOnOneKey { + refuse: &'static str, + stored: std::sync::Mutex>, + } + + #[async_trait::async_trait] + impl crate::ports::SecretStore for FailsOnOneKey { + async fn get( + &self, + _company: &CompanyId, + key: &str, + ) -> crate::error::Result> { + Ok(self + .stored + .lock() + .expect("lock") + .get(key) + .map(|value| SecretValue(value.clone()))) + } + + async fn set( + &self, + _company: &CompanyId, + key: &str, + value: SecretValue, + ) -> crate::error::Result<()> { + if key == self.refuse { + return Err(crate::error::OpenCompanyError::Store( + "the secret store went away mid-write".into(), + )); + } + self.stored + .lock() + .expect("lock") + .insert(key.to_string(), value.0); + Ok(()) + } + } + + /// A host whose company's secret store refuses to write `refuse`. + async fn state_with_failing_secrets(home: &std::path::Path, refuse: &'static str) -> AppState { + use crate::ports::CompanyStore; + use crate::ports::types::CompanyRecord; + + let id = CompanyId::new("acme"); + let manifest: crate::company::CompanyManifest = ::toml::from_str( + "[company]\nname = \"Acme\"\n[[agent]]\nid = \"ceo\"\nrole = \"Chief\"\n[policy]\nmode = \"full\"\n", + ) + .expect("manifest"); + crate::store::FsCompanyStore::new(home.to_path_buf()) + .save(&CompanyRecord { + id: id.clone(), + manifest: manifest.clone(), + ledger: Vec::new(), + lifecycle: "running".to_string(), + overlay_agents: Vec::new(), + overlay_desk_members: Vec::new(), + overlay_desk_order: Vec::new(), + overlay_desk_tools: Default::default(), + overlay_desks: Vec::new(), + overlay_workflows: Vec::new(), + overlay_budgets: Vec::new(), + overlay_policy: None, + disabled_workflows: Vec::new(), + template_provenance: None, + }) + .await + .expect("save"); + + let secrets = std::sync::Arc::new(FailsOnOneKey { + refuse, + ..Default::default() + }); + let runtime = crate::runtime::RuntimeBuilder::new(home.to_path_buf(), manifest) + .with_id(id.clone()) + .with_secrets(secrets) + .build() + .await + .expect("runtime"); + let state = AppState::new(crate::AppConfig::default()); + state.registry().insert(id, std::sync::Arc::new(runtime)); + state + } + + #[tokio::test] + async fn a_save_that_fails_part_way_stores_nothing_at_all() { + // The module header claims a half-configured company is impossible to + // express by accident. Written one `?` at a time it was not: a store + // that took the API key and then failed on the webhook credential + // answered the operator with an error while keeping the key. The + // credential pair is meaningless apart, so "the save failed" and "the + // key is stored" must not both be true. + let home = ::tempfile::tempdir().expect("tempdir"); + let state = state_with_failing_secrets(home.path(), WEBHOOK_SECRET_KEY).await; + let admin = crate::server::test_support::seed_admin(&state, "acme").await; + + let (status, answer) = call( + &state, + "PUT", + "/api/v1/companies/acme/billing/chargebee", + &admin, + Some(json!({ + "apiKey": "cb_live_supersecret", + "site": "acme-test", + "webhookSecret": "cbuser:cbpass", + })), + ) + .await; + assert!( + status.is_server_error() || status.is_client_error(), + "a failed write must not answer OK: {status} {answer}" + ); + + // Read the store directly. Going through `GET` would prove only that the + // status agrees with itself; what matters is that nothing was left on + // disk for the next request — or the next agent turn — to pick up. + let runtime = state + .registry() + .get(&CompanyId::new("acme")) + .expect("company"); + for key in [API_KEY_SECRET, SITE_SECRET, WEBHOOK_SECRET_KEY] { + let stored = runtime + .secrets() + .get(runtime.id(), key) + .await + .expect("read secret"); + assert!( + stored + .as_ref() + .is_none_or(|value| value.expose().is_empty()), + "{key} survived a failed save: {stored:?}" + ); + } + } + + #[tokio::test] + async fn a_failed_paypal_save_does_not_leave_half_a_credential() { + // Same rule on the PayPal side, where half a credential is worse than + // none: a client id with no secret cannot obtain a token, so the tools + // fail on first use rather than never being wired. + let home = ::tempfile::tempdir().expect("tempdir"); + let state = state_with_failing_secrets(home.path(), CLIENT_SECRET_SECRET).await; + let admin = crate::server::test_support::seed_admin(&state, "acme").await; + + let (status, answer) = call( + &state, + "PUT", + "/api/v1/companies/acme/billing/paypal", + &admin, + Some(json!({ + "clientId": "AY_id", + "clientSecret": "EL_secret", + "environment": "live", + })), + ) + .await; + assert!( + status.is_server_error() || status.is_client_error(), + "a failed write must not answer OK: {status} {answer}" + ); + + let runtime = state + .registry() + .get(&CompanyId::new("acme")) + .expect("company"); + for key in [CLIENT_ID_SECRET, CLIENT_SECRET_SECRET, ENVIRONMENT_SECRET] { + let stored = runtime + .secrets() + .get(runtime.id(), key) + .await + .expect("read secret"); + assert!( + stored + .as_ref() + .is_none_or(|value| value.expose().is_empty()), + "{key} survived a failed save: {stored:?}" + ); + } + } + + #[tokio::test] + async fn a_rolled_back_save_restores_what_was_there_before() { + // Rollback restores the PRIOR value, not "empty". An operator correcting + // a site who hits a store failure must still have the connection they + // had before they touched the form. + let home = ::tempfile::tempdir().expect("tempdir"); + let state = state_with_failing_secrets(home.path(), WEBHOOK_SECRET_KEY).await; + let admin = crate::server::test_support::seed_admin(&state, "acme").await; + let runtime = state + .registry() + .get(&CompanyId::new("acme")) + .expect("company"); + + // A working connection, stored directly so the failing key stays out of it. + for (key, value) in [(API_KEY_SECRET, "cb_original"), (SITE_SECRET, "acme-test")] { + runtime + .secrets() + .set(runtime.id(), key, SecretValue(value.to_string())) + .await + .expect("seed"); + } + + let (status, _) = call( + &state, + "PUT", + "/api/v1/companies/acme/billing/chargebee", + &admin, + Some(json!({ + "apiKey": "cb_replacement", + "site": "acme-live", + "webhookSecret": "cbuser:cbpass", + })), + ) + .await; + assert!(!status.is_success(), "the save failed: {status}"); + + for (key, expected) in [(API_KEY_SECRET, "cb_original"), (SITE_SECRET, "acme-test")] { + let stored = runtime + .secrets() + .get(runtime.id(), key) + .await + .expect("read secret") + .map(|value| value.expose().to_string()); + assert_eq!( + stored.as_deref(), + Some(expected), + "{key} was not restored to what it was before the failed save" + ); + } + } + + #[tokio::test] + async fn a_member_may_read_the_status_but_never_write_a_credential() { + // The split is deliberate. `GET` carries no secret — booleans, the site + // slug, the webhook URL — so a member seeing "not connected" is how they + // know to ask an admin. Writing is another matter: a member who could + // `PUT` here would point the company's invoicing at a Chargebee site + // they control, and one who could `DELETE` could silently stop every + // payment notification. + let home = ::tempfile::tempdir().expect("tempdir"); + let state = state_with_company(home.path()).await; + let member = crate::server::test_support::seed_session( + &state, + "acme", + crate::ports::users::UserRole::Member, + ) + .await; + + for uri in [ + "/api/v1/companies/acme/billing/chargebee", + "/api/v1/companies/acme/billing/paypal", + ] { + let (status, answer) = call(&state, "GET", uri, &member, None).await; + assert_eq!(status, StatusCode::OK, "GET {uri}: {answer}"); + } + + for (method, uri, body) in [ + ( + "PUT", + "/api/v1/companies/acme/billing/chargebee", + Some(json!({"apiKey": "cb_key", "site": "attacker-site"})), + ), + ( + "PUT", + "/api/v1/companies/acme/billing/paypal", + Some(json!({"clientId": "AY_id", "clientSecret": "EL_secret"})), + ), + ( + "DELETE", + "/api/v1/companies/acme/billing/chargebee/key", + None, + ), + ("DELETE", "/api/v1/companies/acme/billing/paypal/key", None), + ] { + let (status, answer) = call(&state, method, uri, &member, body).await; + assert!( + status == StatusCode::FORBIDDEN || status == StatusCode::UNAUTHORIZED, + "{method} {uri} answered {status}: {answer}" + ); + } + + // And nothing the member attempted was written. Read the store + // directly rather than through another principal: the refusals above + // are only worth having if they refused the WRITE, not merely the + // response. + let runtime = state + .registry() + .get(&CompanyId::new("acme")) + .expect("company"); + for key in [ + API_KEY_SECRET, + SITE_SECRET, + WEBHOOK_SECRET_KEY, + CLIENT_ID_SECRET, + CLIENT_SECRET_SECRET, + ENVIRONMENT_SECRET, + ] { + let stored = runtime + .secrets() + .get(runtime.id(), key) + .await + .expect("read secret"); + assert!(stored.is_none(), "{key} was written by a member"); + } + } + + #[test] + fn a_site_is_normalized_from_every_shape_an_operator_pastes() { + for raw in [ + "acme-test", + " acme-test ", + "acme-test.chargebee.com", + "https://acme-test.chargebee.com", + "https://acme-test.chargebee.com/", + "http://acme-test.chargebee.com/", + ] { + assert_eq!(normalize_site(raw), "acme-test", "from {raw:?}"); + } + } + + #[test] + fn an_empty_site_stays_empty_rather_than_becoming_a_url_fragment() { + assert_eq!(normalize_site(""), ""); + assert_eq!(normalize_site(" "), ""); + assert_eq!(normalize_site("https://"), ""); + } + + #[test] + fn status_never_serializes_a_credential() { + // The whole contract of this module: whatever else changes, no field + // here may carry the key. Asserted on the serialized form, because that + // is what actually reaches a browser. + let status = BillingStatus { + api_key_configured: true, + site: Some("acme-test".to_string()), + webhook_configured: true, + webhook_url: Some("https://oc.example/hooks/acme/chargebee".to_string()), + granted: true, + in_build: true, + }; + let json = serde_json::to_string(&status).expect("serializes"); + assert!(json.contains("apiKeyConfigured")); + assert!(json.contains("acme-test")); + // No field may be named in a way that could carry the secret itself. + assert!(!json.contains("apiKey\""), "{json}"); + assert!(!json.contains("webhookSecret"), "{json}"); + } + + #[test] + fn a_paypal_status_never_serializes_a_credential() { + let status = PaypalStatus { + client_id_configured: true, + client_secret_configured: true, + environment: "sandbox".to_string(), + granted: true, + in_build: true, + }; + let json = serde_json::to_string(&status).expect("serializes"); + assert!(json.contains("clientIdConfigured")); + assert!(json.contains("sandbox")); + // No field may carry either half of the credential itself. + assert!(!json.contains("clientId\""), "{json}"); + assert!(!json.contains("clientSecret\""), "{json}"); + } + + #[test] + fn the_three_failure_modes_stay_distinguishable() { + // Credentials present, grant missing: the operator's remedy is the + // manifest, not the settings form. Collapsing these into one + // "connected" boolean is what sends them to the wrong place. + let configured_but_ungranted = BillingStatus { + api_key_configured: true, + site: Some("acme-test".to_string()), + webhook_configured: false, + webhook_url: None, + granted: false, + in_build: true, + }; + assert!(configured_but_ungranted.api_key_configured); + assert!(!configured_but_ungranted.granted); + assert!(!configured_but_ungranted.webhook_configured); + } +} diff --git a/src/server/ops/capabilities.rs b/src/server/ops/capabilities.rs index b273a1204..d21254db8 100644 --- a/src/server/ops/capabilities.rs +++ b/src/server/ops/capabilities.rs @@ -76,6 +76,15 @@ struct CapabilityStatusDto { composio_granted: bool, /// Whether the `composio` feature is compiled into this build at all. composio_in_build: bool, + /// Chargebee billing (issue #788): whether this company **explicitly** grants + /// the `chargebee` namespace (a `*` wildcard does NOT count). What the + /// Settings UI reads to say whether billing tools would reach an agent even + /// once credentials are saved. + chargebee_granted: bool, + /// Whether the `chargebee` feature is compiled into this build at all. The + /// grant and the credentials can both be in place and still wire no tools if + /// the running binary was not built with it. + chargebee_in_build: bool, /// Whether a non-empty per-tenant Composio **BYO override** token is stored /// under `composio/token` — never the token itself. Unlike media's env /// credential, this is a tenant secret. @@ -188,6 +197,7 @@ struct TotalDto { /// composio), independent of whether a `[plan]` is configured. struct OptInFlags { media_granted: bool, + chargebee_granted: bool, composio_granted: bool, composio_token_configured: bool, /// The resolved Composio credential tier (issue #886), or `None` when it @@ -207,6 +217,7 @@ impl OptInFlags { fn none() -> Self { Self { media_granted: false, + chargebee_granted: false, composio_granted: false, composio_token_configured: false, // `None` (undetermined), never `Some(CredentialSource::None)`: @@ -236,6 +247,8 @@ fn unconfigured(flags: OptInFlags) -> CapabilityStatusDto { media_credential_configured: media_credential_configured(), composio_granted: flags.composio_granted, composio_in_build: cfg!(feature = "composio"), + chargebee_granted: flags.chargebee_granted, + chargebee_in_build: cfg!(feature = "chargebee"), composio_token_configured: flags.composio_token_configured, composio_credential_source: flags.composio_credential_source, search_granted: flags.search_granted, @@ -340,6 +353,7 @@ async fn effective_status(runtime: &CompanyRuntime) -> Result Result Router { .merge(tool_catalog::router()) .merge(connections_read::router()) .merge(channels::router()) + .merge(billing::router()) .merge(company_key::router()) .merge(composio::router()) .merge(domain::router()) diff --git a/src/server/routes.rs b/src/server/routes.rs index 3bb0d39eb..324bd309e 100644 --- a/src/server/routes.rs +++ b/src/server/routes.rs @@ -74,6 +74,7 @@ fn router_with_console(state: AppState, console_dir: Option) -> Router .merge(crate::server::operator::router()) .merge(crate::server::ops::router()) .merge(crate::server::hooks::router()) + .merge(crate::server::hooks_chargebee::router()) .merge(crate::server::provision::router()) .merge(crate::server::setup::router()) .merge(crate::server::feedback::router()) diff --git a/src/workflows/gated_tool_turn_test.rs b/src/workflows/gated_tool_turn_test.rs index b08e52abe..2ed338364 100644 --- a/src/workflows/gated_tool_turn_test.rs +++ b/src/workflows/gated_tool_turn_test.rs @@ -238,6 +238,10 @@ pub(super) fn deps(base_url: String, dir: &std::path::Path) -> (HarnessDeps, Arc plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), run_supervisor: crate::runtime::RunSupervisor::default(), delivery: Some(WorkflowDeliveryDeps { diff --git a/src/workflows/runner.rs b/src/workflows/runner.rs index d819c361e..e76fe59d8 100644 --- a/src/workflows/runner.rs +++ b/src/workflows/runner.rs @@ -1445,6 +1445,10 @@ description = "Runs Acme." plan: None, media: None, composio: None, + #[cfg(feature = "chargebee")] + chargebee: None, + #[cfg(feature = "paypal")] + paypal: None, steer: crate::company::steer::InflightRegistry::default(), delivery: None, search: None,