services: add path-based matching with name as canonical identifier

[dangtony98]

Summary

• Services gain optional URL-path scoping, so the same host can carry multiple credentials at different paths (motivating case: Slack with /api/* for the Bot token and /api/apps.connections.* for Socket Mode).
• The host-only matcher is replaced by deterministic specificity scoring: exact-host beats wildcard-host, then longer literal path prefix wins, then declaration order.
• A slugged name becomes the canonical per-vault service identifier, fixing a long-standing weakness where two services on the same host couldn't be addressed independently.

Highlights

• broker.MatchService(host, path, services) replaces MatchHost. Returns (*Service, MatchScore) so the debug log can audit (host_tier, path_prefix_len, decl_order) without recomputing.
• DELETE/PATCH /v1/vaults/{vault}/services/{name-or-host}: tries name first, falls back to host, returns 409 with the candidate list on ambiguity. Single-rule callers see no behavior change.
• Inline-form host (slack.com/api/*) is split server-side at every ingest path; CLI mirrors the split for --host.
• Auto-naming: services without name are slugged via broker.Slugify(host, path) at write time. Legacy services are normalized lazily on read — no migration step, no manual config edits, no DB schema change.
• broker.ValidateSlug consolidates the 3–64 lowercase-alphanumeric-hyphens rule that previously lived in a private validateSlug shared by vault and agent name handlers.
• TypeScript SDK v0.2.0 (additive, non-breaking): optional name?/path? on Service, new removeByName(name) method, existing remove(host) still works against the back-compat host-shim route.

Breaking changes (operator-visible)

• ProxyEvent.MatchedService semantic flip: previously stored the matched host string; now stores the canonical service name (slug). New MatchedHost/MatchedPath fields carry the matched pattern alongside (in-memory only — no schema change). Documented in cmd/skill_http.md. Operators filtering request logs by ?service= should switch to the slug; pre-upgrade rows still contain hosts.

Test plan

• make test — full Go suite passes (only pre-existing TestParseAndValidateMount_RejectDockerSocket in internal/isolation fails, environment-dependent on a local docker socket; unrelated to this change)
• SDK tests pass: cd sdks/sdk-typescript && npm test (98/98)
• Frontend typechecks: cd web && npx tsc --noEmit
• go vet ./... clean
• make build produces a working binary end-to-end
• New broker matcher tests pin specificity scoring, host-tier-beats-wildcard, glob cross-/, declaration-order tiebreak, port-stripping, empty-path catch-all
• New server-level tests pin 409 on host ambiguity (DELETE + PATCH), name-resolution happy path, and inline-form HTTP ingest
• New SDK tests cover name/path round-trip and removeByName
• Manual UI smoke (reviewer): create two Slack services through the form (one via inline-form paste slack.com/api/*, one via separate Host + Path fields), toggle one off, confirm the other still proxies, list shows distinct rows keyed by name
• Manual end-to-end Slack (reviewer): agent-vault vault service add --name slack-bot --host slack.com --path '/api/*' …, then --name slack-conn --host slack.com --path '/api/apps.connections.*' …, send requests through the proxy and confirm the right credential is injected for each (visible in vault run debug logs as service=slack-bot|slack-conn)

[infisical-review-police]

💬 Discussion in Slack: #pr-review-agent-vault-164-services-add-path-based-matching-with-name-as-canonica

Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
agent-vault	🟢 Ready	View Preview	May 10, 2026, 2:45 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[dangtony98]

@claude review

[dangtony98]

@claude review

[dangtony98]

@claude review

[claude]

🔴 Approving any proposal on a pre-PR vault panics the request goroutine. Legacy services persisted before this PR have Name="" (the field is new in this diff), and MergeServices's new guards (merge.go:15-25) fire on the first existing entry. The PR description claims "Legacy services are normalized lazily on read," but broker.NormalizeServices and broker.Slugify don't exist anywhere in this codebase — loadServices (handle_services.go:134-150) only applies SplitInlineHost, never backfilling Name. Fix: implement the missing normalizer and call it in loadServices so existing entries are backfilled before reaching MergeServices, or replace the panic with a returned error surfaced as a 500 with a migration message. The Web UI's PUT-services edit path is also broken for the same root cause (broker.Validate hard-rejects empty Name at broker.go:374-376), so operators have no in-product remediation.

Extended reasoning...

What the bug is

Legacy services persisted before this PR have no Name field on the struct (the diff shows Description *string being removed and Name string being added on broker.Service). When json.Unmarshal runs against a pre-PR services_json row, every legacy entry materialises with Name="". This PR adds new panic guards at the top of proposal.MergeServices (merge.go:15-25):

for i, s := range existing {
    if s.Name == "" {
        panic(fmt.Sprintf("proposal.MergeServices: existing[%d] has empty Name (host=%q)", i, s.Host))
    }
}
for i, p := range proposed {
    if p.Name == "" {
        panic(fmt.Sprintf("proposal.MergeServices: proposed[%d] has empty Name (host=%q, action=%q)", i, p.Host, p.Action))
    }
}

The PR description and the docstring on MergeServices both say "callers must normalize before MergeServices" / "Legacy services are normalized lazily on read." That contract is unimplemented.

The missing helpers

grep -rn 'NormalizeServices\|Slugify' across the entire repository (Go, MDX, MD, TS, TSX) returns zero matches. The PR description, several review comments, and the docstring on MergeServices reference these helpers as if they exist, but only the call sites that depend on them landed — not the helpers themselves. Functions actually present in internal/broker/broker.go are Validate, MatchService, ValidateSlug, ValidatePath, ValidateHost, SplitInlineHost. None of them backfill Name.

The specific code path that triggers it

handleAdminProposalApprove (handle_proposals.go:478-493) runs:

1. existingServices, err := s.loadServices(ctx, ns.ID) — returns legacy entries with Name="".
2. proposedServices, err = normalizeProposalServices(proposedServices, existingServices) — only fills Name for ActionDelete (handle_services.go:71-83); ActionSet entries with empty Name pass through unchanged. Even on ActionDelete, when the matched existing service is legacy, the adopted Name is still "".
3. merged, _ := proposal.MergeServices(existingServices, proposedServices) — panics on the first iteration of the first guard loop at merge.go:17, before any merge work happens.

I verified loadServices end-to-end: it calls GetBrokerConfig, unmarshals into []broker.Service, applies SplitInlineHost per entry, returns. No name normalization step exists. I verified no migration was added — internal/store/migrations/ ends at 045_no_access_role.sql; nothing touches broker_configs.services_json. I verified there is no recover() anywhere in internal/server/, so Go's net/http default panic handler closes the connection and the operator sees a 500 / dropped connection with the stack trace in logs.

Step-by-step proof (legacy vault)

1. Operator upgrades to this PR. Vault holds [{"host":"api.stripe.com","auth":{...}}, {"host":"*.github.com","auth":{...}}] from before — JSON has no "name" key.
2. Agent submits a fresh proposal post-upgrade with a valid Name field: {"services":[{"action":"set","name":"slack-bot","host":"slack.com","auth":{...}}], ...}. Create succeeds.
3. Admin clicks Approve. handleAdminProposalApprove runs. loadServices returns the two legacy entries, both with Name="". normalizeProposalServices returns the proposed slice unchanged (the new entry already has Name="slack-bot").
4. proposal.MergeServices(existingServices, proposedServices) enters the first guard loop. At i=0, existing[0].Name == "" — panic fires with proposal.MergeServices: existing[0] has empty Name (host="api.stripe.com").
5. The goroutine's stack trace gets dropped to logs; net/http's connection handler catches it and the operator sees a connection drop / 500. The proposal cannot be approved — there is no UI workaround.

Web UI breakage (same root cause)

The Web UI's PUT-services edit flow is also fully broken for legacy vaults. GET /v1/vaults/{name}/services returns legacy entries with name="" because the read path doesn't backfill either. Saving any edit via the ServicesTab modal triggers PUT /services with the full list (ServicesTab.tsx:saveServices), which calls broker.Validate server-side — and broker.go:374-376 hard-rejects every empty-Name entry: return fmt.Errorf("service %d: name is required", i). Operators cannot edit a single service without first manually filling Names for every legacy service. There is no UI path that backfills automatically.

Why this is normal severity and how to fix

Severity is normal (not critical) because Go's net/http catches the panic at the connection level so the process keeps running, and brand-new installs with no legacy data are unaffected. But the trigger is a routine operator action (approve a proposal) on the headline data shape this PR claims to support, and there is no in-product remediation path. The fix the PR description implies is missing: implement Slugify(host, path) (a deterministic, idempotent slug builder) plus NormalizeServices(svcs []Service) []Service that backfills Name = Slugify(Host, Path) (with collision-bumping if needed) for any entry with empty Name, and call it in loadServices so every read path produces normalized entries. Alternatively/additionally: replace the panic in MergeServices with a returned error, and have the apply path surface it as a 500 with a clear migration message — at least no crash. The first option restores the documented "lazy normalization on read" contract; the second is a defense-in-depth backstop for the data-loss class concern the panic docstring cites.

[claude]

🟡 The TypeScript SDK type ServiceInput.name at sdks/sdk-typescript/src/resources/services.ts:99 is declared name?: string (optional), but the JSDoc literally one line above says "Required on write" and the server hard-rejects missing Name with 400 (broker.Validate at internal/broker/broker.go:353-355: service N: name is required). A TypeScript user can write vault.services!.set([{host:"api.stripe.com", auth:{...}}]) and tsc --strict compiles cleanly — the runtime failure is the first signal. The same mismatch exists on WireService.name at sdks/sdk-typescript/src/types.ts:108-109. Fix: drop the ? on both interfaces so the type matches the contract.

Extended reasoning...

What the bug is

The SDK exports two write-side types whose name field is optional in the type system but mandatory at runtime:

• ServiceInput.name?: string at sdks/sdk-typescript/src/resources/services.ts:99 — the JSDoc directly above on line 98 says "Required on write". This is the write shape consumed by ServicesResource.set() and replaceAll().
• WireService.name?: string at sdks/sdk-typescript/src/types.ts:108-109 — the JSDoc says "Required on write — pick deliberately; the server does not derive it from host". This is the internal wire type.

The server enforces the contract hard at broker.Validate (internal/broker/broker.go:353-355):

if s.Name == "" {
    return fmt.Errorf("service %d: name is required", i)
}

Surfaced by handleServicesUpsert as HTTP 400 with Invalid services: service 0: name is required.

Why it matters

Every other write-required field on ServiceInput is non-optional in the type — host: string, auth: ServiceAuth. Optional fields are correctly typed optional — enabled?, substitutions?. The lone outlier is name?, where prose says required but the type system says optional. TypeScript strict mode protects against typos in object literals (e.g. the description excess-property error users hit), but it cannot protect against a field whose own type signature permits omission.

Step-by-step proof

1. TypeScript user installs @infisical/agent-vault-sdk@0.2.0, opens ServiceInput in their IDE, sees name?: string.
2. Following the visible type, they write:

   await vault.services!.set([{
     host: "api.stripe.com",
     auth: { type: "bearer", token: "STRIPE_KEY" },
   }]);

3. tsc --strict compiles cleanly — name? permits omission.
4. SDK marshals {services:[{host:"api.stripe.com",auth:{...}}]} and POSTs to /v1/vaults/{name}/services.
5. Server handleServicesUpsert runs broker.Validate(&incoming). At broker.go:353 the check fires.
6. Server returns HTTP 400 {"error":"Invalid services: service 0: name is required"}.
7. The user got no compile-time signal — only a runtime 400 on a field they were never told they had to fill in.

Why the JSDoc is not enough

JSDoc renders in IDE hover tooltips but does not constrain the build. The strict-mode excess-property check that catches typos on the user-supplied side does NOT engage for missing optional fields. The type system is what users trust; the JSDoc is the second source of truth at best.

Why this is nit, not normal

The runtime behavior is correct (server returns a clear 400 with an actionable message), no data loss, no security risk. But this is the v0.2.0 release whose headline feature is exactly "name is canonical" — the SDK's type system is the first line of error feedback for users adopting the new model, and a self-contradicting type signature on the canonical write method undermines the migration story this release exists to deliver. Fix is one character per location.

How to fix

Drop the ? in both files:

 // sdks/sdk-typescript/src/resources/services.ts:99
-  name?: string;
+  name: string;

 // sdks/sdk-typescript/src/types.ts:109
-  name?: string;
+  name: string;

Existing callers that supply name are unaffected. Callers omitting name now fail at compile time with TS2741: Property 'name' is missing in type '{ host: string; auth: ...; }' but required in type 'ServiceInput'. — immediate, actionable feedback matching the server contract.

[claude]

🟡 The fallback error message printed when vault proposal create is invoked with no flags (cmd/proposal_create.go:86) shows an example that copies-and-pastes into an immediate failure: --host api.example.com is given without the now-required --name flag, so the user hits --name is required when --host is specified from buildFromFlags. The Long help block (lines 27-45) was correctly migrated to include --name my-service in every --host example; this parallel error-fallback string was missed. Fix is one token: insert --name my-service before --host in the second example on line 86.

Extended reasoning...

What the bug is

The error returned at cmd/proposal_create.go:86 (when the user runs agent-vault vault proposal create with no -f, no --host, and no --credential) prints three examples to guide the user:

provide either --host/--credential flags or -f <file>

Examples:
  agent-vault vault proposal create --credential MY_KEY="description" --message "reason"
  agent-vault vault proposal create --host api.example.com --auth-type bearer --token-key MY_KEY --credential MY_KEY --message "reason"
  agent-vault vault proposal create -f proposal.json

The second example is broken on this PR: it sets --host api.example.com but omits --name. This PR introduced a hard requirement in buildFromFlags (cmd/proposal_create.go:194-197):

name, _ := cmd.Flags().GetString("name")
if name == "" {
    return nil, fmt.Errorf("--name is required when --host is specified")
}

A user who copies the example verbatim hits this check immediately and gets an error referencing a flag that was not in the help they just read.

Why existing code does not prevent it

The Long help block at cmd/proposal_create.go:27-45 was correctly updated in this PR to include --name in every --host example (e.g. --name stripe --host api.stripe.com, --name slack-bot --host 'slack.com/api/*'). The author was aware of the new requirement and migrated the canonical docs surface — but missed this second string constant at line 86, which is a separate piece of inline text only visible when the user provides no flags at all. The asymmetry between two places in the same file is concrete evidence this is an oversight rather than a deliberate choice.

Step-by-step proof

1. User runs agent-vault vault proposal create with no flags.
2. Lines 79-86 hit the else branch (no filePath, no host, no credentials), returning the error string at line 86.
3. User reads the output and copies the second example verbatim: agent-vault vault proposal create --host api.example.com --auth-type bearer --token-key MY_KEY --credential MY_KEY --message "reason".
4. The new invocation reaches buildFromFlags (cmd/proposal_create.go:164), which builds the auth config and then runs the --name check at line 194-197.
5. cmd.Flags().GetString("name") returns "" (the flag was never passed); buildFromFlags returns --name is required when --host is specified.
6. The proposal create aborts; the user sees an error about a flag that was not present in the example they just copied.

Impact

Documentation-only — no runtime regression, no data loss, no security risk. Severity is nit because the user gets a clear error and can recover by adding --name. But the printed examples are a copy-paste invitation and currently guide users straight into a contradiction with a contract this PR newly introduced. The Long help that was correctly updated in this PR already demonstrates the right shape (--name slack-bot --host 'slack.com/api/*'), so the fix is unambiguous.

How to fix

One-token change at line 86 — insert --name my-service before --host in the second example:

-  agent-vault vault proposal create --host api.example.com --auth-type bearer --token-key MY_KEY --credential MY_KEY --message "reason"
+  agent-vault vault proposal create --name my-service --host api.example.com --auth-type bearer --token-key MY_KEY --credential MY_KEY --message "reason"

This matches the convention the Long help block at lines 27-45 already uses for every --host example.