feat(metering): add external metering filter with balance checks and usage reporting

[noyitz]

Adds an external_metering HTTP filter: pre-request balance checks and
post-response token usage reporting against an external metering service.

Part of #577.

What the filter does

Request phase — resolves tenant identity, strips identity headers and
client credentials (authorization, x-api-key), and asks the metering
service whether the tenant may spend more tokens:

• GET {metering_url}/api/v1/customers/{username}/entitlements/{feature_key}/value?model={model}
• Exhausted budget → 429. Metering unreachable → admit when fail_open: true
  (default), 503 otherwise, so a metering outage degrades to unmetered service
  rather than an inference outage.

Response phase — emits a CloudEvents 1.0 event
(inference.tokens.used, or inference.request.error on upstream failure) to
POST {metering_url}/api/v1/events. Delivery is fire-and-forget: a slow or
failing metering service never delays a response the upstream already
answered. Token counts come from the token_count filter's filter_metadata
keys (token.input/output/total and the prompt cache breakdown from #582).

Identity resolution — three tiers, most trusted first

1. Verified metadata (unnamespaced {prefix}* keys) written by an
   authentication filter from verified credentials. When present, every lower
   tier is ignored entirely, so a client cannot spoof subscription or
   model via forged headers alongside valid credentials.
2. Guard metadata (namespaced identity.{prefix}* keys) written by the
   identity_header_guard filter (feat(filter): add identity header guard filter #709), which owns identity header capture.
3. Raw {prefix}* headers, for deployments where a trusted upstream auth
   layer (e.g. an external authorizer) injects them directly.

Identity headers are always stripped before the request reaches the upstream,
regardless of which tier supplied the identity.

Relationship to #709 and #577

This is the metering half of the split: #709 owns identity header capture into
namespaced metadata; this filter consumes that metadata (tier 2) and owns
admission control, credential stripping, and usage reporting. #577 documents
the original single-filter design and needs an update to reflect the split.

HTTP callouts use praxis-core's SubRequestClient (the CalloutClient named
in #577 was removed upstream in #849). register_ai_filters threads the shared
server-level client into the filter following the existing
openai_file_resolve pattern; without one the filter creates a private
connector, same as openai_file_resolve::from_config.
praxis_ai_apis::subrequest::execute_url is made public so filters can execute
full-URL sub-requests.

Size

1,876 insertions, of which 861 are unit + integration tests, 103 generated
docs + README rows, and 64 the example config. Functional filter code is
~1,010 lines across mod.rs, config.rs, and registration. Balance check,
event construction, and identity resolution are cohesive enough that splitting
them again would leave non-functional intermediate states; happy to split if
reviewers prefer.

Testing

• 41 unit tests (filters/src/metering/tests.rs), including spoofing-closure
  tests for the tier precedence rules
• 5 functional integration tests against examples/configs/external-metering.yaml
  (balance allowed, fail-closed rejection, header stripping, no-identity skip)
• make test — 4,905 passed, 0 failed; make lint green

[praxis-bot]

Review Summary

Clean, well-structured filter with good test coverage and correct conventions. Three medium-severity items found.

Severity	Count
Critical	0
Large	0
Medium	3

Findings:

1. [Medium] strip_client_credentials uses if let Ok for an infallible HeaderName parse, creating a silent-failure pattern for security-critical credential stripping. Use HeaderName::from_static("x-api-key") instead.
2. [Medium] validate_config only checks for empty prefix but does not validate that the prefix contains valid HTTP header name characters. An invalid prefix (e.g. containing spaces or control characters) would silently match no headers, leaving tenant identity headers unstripped.
3. [Medium] No test covers multi-value tenant headers (x-tenant-username: alice\r\nx-tenant-username: mallory). Add a unit test verifying all values are marked for removal when a client sends duplicate tenant headers.

[praxis-bot]

[Medium] "x-api-key".parse::<HeaderName>() is infallible for this input, but if let Ok silently swallows a hypothetical failure. In a credential-stripping function, silent failure to remove a header is a security concern. Replace with HeaderName::from_static("x-api-key") which is compile-time validated and makes the intent unambiguous:

fn strip_client_credentials(ctx: &mut HttpFilterContext<'_>) {
    ctx.request_headers_to_remove.push(http::header::AUTHORIZATION);
    ctx.request_headers_to_remove
        .push(HeaderName::from_static("x-api-key"));
}

[noyitz]

@praxis-bot fixed in 79c3709 — HeaderName::from_static("x-api-key"), compile-time validated, no silently-swallowed branch.

[praxis-bot]

[Medium] This validates non-empty but not that the prefix consists of valid HTTP header name characters (RFC 7230 tchar). A misconfigured prefix like "x tenant " or "x-tenant\t" would silently match no headers, leaving tenant identity unstripped while the filter reports healthy. Add a validation that the lowercased prefix can form a valid HeaderName prefix, e.g.:

if HeaderName::from_bytes(cfg.identity_header_prefix.as_bytes()).is_err()
    && HeaderName::from_bytes(
        format!("{}test", cfg.identity_header_prefix).as_bytes(),
    ).is_err()
{
    return Err("external_metering: identity_header_prefix contains invalid header name characters".into());
}

Or simply attempt HeaderName::from_bytes(format!("{prefix}x").as_bytes()) and reject on error, since the prefix must be combinable with a suffix to form a valid header name.

[noyitz]

@praxis-bot fixed in 79c3709 — validate_config now rejects any prefix that is not a valid HTTP header name via HeaderName::from_bytes, with a test for a prefix containing spaces.

[praxis-bot]

[Medium] Missing test for multi-value tenant headers. A client could send x-tenant-username: alice and x-tenant-username: mallory on the same request. Add a test that inserts duplicate values for the same tenant header key and asserts the key appears in request_headers_to_remove (confirming all values are targeted for removal).

[noyitz]

@praxis-bot added in 79c3709 — multi_value_identity_header_is_fully_stripped appends a duplicate x-tenant-username and asserts the name lands in request_headers_to_remove (removal by name drops every value).

[praxis-bot]

Review Summary

Clean, well-structured filter with good test coverage and correct conventions. The prior review covered three medium items; this pass adds one more.

Severity	Count
Critical	0
Large	0
Medium	1

Findings:

1. [Medium] strip_identity_headers performs two unnecessary per-request allocations: lowering HeaderName::as_str() (already lowercase by http crate contract) and lowering the immutable prefix on every call instead of once at construction.

[praxis-bot]

[Medium] HeaderName::as_str() already returns lowercase (the http crate normalizes header names on construction), so to_ascii_lowercase() on the key is a redundant String allocation per header per request. Additionally, the prefix is immutable after construction, so its lowercase form should be computed once in build() rather than on every request.

Pre-lower the prefix at construction:

Ok(Self {
    identity_header_prefix: cfg.identity_header_prefix.to_ascii_lowercase(),
})

Then simplify the hot-path function to zero allocations:

fn strip_identity_headers(ctx: &mut HttpFilterContext<'_>, prefix: &str) {
    for key in ctx.request.headers.keys() {
        if key.as_str().starts_with(prefix) {
            ctx.request_headers_to_remove.push(key.clone());
        }
    }
}

The header_matching_ignores_case test continues to pass because the http crate already lowercases the header name at insertion and the prefix is now pre-lowered at init.

[noyitz]

@praxis-bot fixed in 79c3709 — the prefix is lowercased once at construction and compared against HeaderName::as_str() directly; the per-header to_ascii_lowercase() allocation is gone.

[praxis-bot]

No new findings beyond prior review.

[praxis-bot]

No new findings beyond prior reviews.

Fourth pass confirms the four existing findings (infallible HeaderName parse, prefix validation gap, missing multi-value header test, redundant to_ascii_lowercase) are the substantive items. Code structure, conventions, test coverage, integration tests, documentation, and registry wiring are all correct.

[praxis-bot]

Review Summary

Fifth pass. One new medium finding (assertion messages in unit tests). Four items from prior reviews remain unaddressed in the code.

Severity	Count (new)	Count (prior, open)
Critical	0	0
Large	0	0
Medium	1	4

New finding:

1. [Medium] Unit test assertions missing messages (tests.rs). All 15 assert!/assert_eq! calls lack a message string. The workspace enforces missing_assert_message = "deny" (Cargo.toml line 223), and the test module's #[allow] only covers unwrap_used and expect_used. The integration tests correctly include messages on every assertion.

Open prior findings (no code changes since last review):

1. HeaderName::from_static("x-api-key") instead of fallible .parse() with if let Ok (mod.rs:102)
2. Prefix validation for valid HTTP header characters (config.rs:30)
3. Missing multi-value tenant header test (tests.rs)
4. Redundant per-request to_ascii_lowercase on both key and prefix (mod.rs:88)

[praxis-bot]

[Medium] All 15 assert!/assert_eq! calls in the unit tests omit assertion messages. The workspace enforces missing_assert_message = "deny" (Cargo.toml line 223), and the test module's #[allow] only covers unwrap_used and expect_used — it does not suppress missing_assert_message.

The integration tests correctly include messages on every assertion. The unit tests should follow the same practice for consistent diagnostics on failure.

Example fix for this line:

assert_eq!(filter.name(), "external_metering", "filter name should be external_metering");
assert_eq!(filter.identity_header_prefix, "x-tenant-", "default prefix should be x-tenant-");

[noyitz]

@praxis-bot make lint (clippy --workspace --all-targets -D warnings) is green on this branch, so the workspace deny does not fire on these asserts as written. All tests added since carry messages; happy to sweep the earlier ones too if maintainers want it in this PR.

[leseb]

@noyitz please resolve the conflicts and address the bot's comments thanks

[jordigilh]

A few things flagged inline, plus some broader ones here. Identity stripping, the CloudEvents payload, and URL-encoding of client-influenced path segments all held up -- no SSRF or credential-leakage concerns.

Circuit breaking is available but unused for this gateway. SubRequestConnector supports one (with_options, praxis#879) and the base proxy wires it up; praxis-ai's server never does (server.rs#L141, commands.rs#L40 both use plain ::new()). runtime.subrequest_circuit_breaker parses fine in an AI-gateway config today and is silently dropped. external_metering is the first mandatory-every-request consumer of that shared pool (the other 5 are opportunistic) -- worth a follow-up before a high-traffic rollout.

execute_url (+ client/error types) are now pub, not pub(crate) -- needed cross-crate, but worth flagging as a public API addition on praxis-ai-apis, semver-relevant going forward.

Generated docs skip the 3-tier identity design -- it only lives in a doc comment on the private read_identity_headers, which the doc generator never harvests (it reads a second paragraph of the module/struct doc -- credential_inject.md already does this). Worth moving it up.

#577 (this filter's linked design doc) still describes the single-filter/CalloutClient design this PR (split across #581 + #709) supersedes.

Minor: iterative_request_router's validate() doesn't block nesting external_metering inside a step (it does block nested IRR and compression) -- would double-report with no idempotency key if misconfigured. Not urgent, just flagging.

[jordigilh]

Any non-2xx from the metering service (403/404/500) hits the same on_metering_unavailable() path as a real outage (L239-242) -- under the default fail_open: true, an explicit denial gets silently admitted. Is a 4xx from this API ever meant to signal denial vs. unavailability? If so, worth splitting the two paths.

[noyitz]

@jordigilh good question — checked the service contract: the metering service expresses denial only through a 2xx response carrying hasAccess: false (its only non-2xx is a 400 for a malformed path), so a 4xx here always means the service itself is misbehaving, never an intentional denial. 79c3709 documents that contract on check_balance and raises the non-2xx log to warn! with the status so a misbehaving service is visible. If a future backend wants status-coded denials, splitting the paths would be a config-gated follow-up.

[jordigilh]

has_verified_identity only checks username (L554) -- if tier 1 sets group/subscription/model without username, tier 2 unconditionally overwrites those verified fields. Not just theoretical: jwt_auth (#769) sets each claim independently, and its validate_config doesn't require a username mapping. Worth gating on "any tier-1 field present" instead.

[noyitz]

@jordigilh fixed in 79c3709 — the gate is now "any verified field present" (username, group, subscription, or model), so an auth filter that maps only some claims (exactly the #769 case) blocks the lower tiers entirely. Covered by a new test: a tier-1 group-only identity rejects both a guard-metadata subscription and a raw-header username.

[jordigilh]

"identity." is hardcoded here, but identity_header_guard (#709) makes that namespace configurable (default "identity"). Works today only because both default -- if an operator changes it, tier 2 silently stops resolving with no fallback. Worth a matching config field.

[noyitz]

@jordigilh fixed in 79c3709 — new identity_metadata_namespace config field (default identity), validated non-empty, with a test resolving tier 2 through a custom namespace. The rustdoc notes it must match identity_header_guard's metadata_namespace.

[jordigilh]

Only signal on delivery failure is a debug! line -- no metric, and SubRequestClient doesn't emit one either. A metering blip during a traffic spike would be invisible. A metrics::counter! here (already a dependency) would fix that.

[noyitz]

@jordigilh fixed in 79c3709 — added praxis_ai_metering_report_failures_total, incremented on both transport failure and non-2xx acknowledgement, and raised those logs to warn!. Naming follows the existing praxis_ai_ttft_seconds convention.

[jordigilh]

Substring scan for "model" rather than JSON parsing -- a content field containing "model": ahead of the real field gets misattributed. Confirmed reachable:

let body = br#"{"messages":[{"role":"user","content":"hi","model":"decoy"}],"model":"gpt-4"}"#;
assert_eq!(extract_model_from_bytes(body).as_deref(), Some("decoy")); // should be "gpt-4"

token_count already parses this correctly (serde_json::from_slice) -- worth reusing that approach.

[noyitz]

@jordigilh confirmed and fixed in 79c3709 — replaced the substring scan with a string- and depth-aware scanner that only matches a "model" key belonging to the top-level object, still without buffering or deserializing the document. Your repro is now a test (extract_model_ignores_nested_decoy), plus decoy-inside-string-value and nested-only-decoy cases.

[jordigilh]

This ordering requirement isn't enforced anywhere -- praxis-core's FilterPipeline::ordering_errors() hard-fails at startup, but only checks core's own built-in filter names, not cross-crate ai filters. Wrong order -> every event ships with token: 0, silently, forever.

[noyitz]

@jordigilh agreed — core's ordering_errors() can't see ai-crate filters, so this is only documented today. Rather than a filter-local workaround, this seems worth a small upstream mechanism (filters declaring ordering constraints the pipeline validates at build). Happy to open a follow-up issue proposing that if you agree.

[noyitz]

@jordigilh thanks for the thorough pass — all six inline findings are addressed in 79c3709 (replied on each thread), make test 4,913 green, lint green. On the broader notes:

• Circuit breaker: agreed this filter is the first mandatory-per-request consumer of the shared pool. Wiring SubRequestConnector::with_options through the server (so runtime.subrequest_circuit_breaker stops being silently dropped) deserves its own PR — happy to open the follow-up issue.
• execute_url going public: acknowledged as a deliberate API addition on praxis-ai-apis; the alternative was duplicating the URL/DNS/peer-fallback logic in the filters crate.
• Docs: the three-tier identity design now lives in the struct documentation, so the generated external_metering.md carries it (regenerated in 79c3709).
• feat(filter): add external metering filter for usage reporting and balance checks #577: updating it now to reflect the feat(metering): add external metering filter with balance checks and usage reporting #581 + feat(filter): add identity header guard filter #709 split and the CalloutClient → SubRequestClient migration.
• IRR nesting: agreed, will fold an external_metering guard into the same follow-up as the ordering-validation idea.

[praxis-bot]

Re-review (6th pass)

All four code findings from prior reviews have been addressed:

• HeaderName::from_static("x-api-key") (fixed, line 667)
• Prefix validation for valid header characters (fixed, config.rs lines 99–106)
• Multi-value identity header test (added, tests.rs lines 411–426)
• Redundant per-request to_ascii_lowercase (pre-lowered at construction, line 229)

Assertion messages (R5) are partially addressed: security and spoofing tests now include messages, config/parsing tests still lack them.

One new medium finding below.

Severity	Count (new)
Critical	0
Large	0
Medium	1

[praxis-bot]

[Medium] Query-string parameter injection: the model value is percent-encoded with PATH_SEGMENT, which does not encode & (0x26) or = (0x3D). Since the model is interpolated into a query parameter (?model={model}), a model name containing & (e.g. a custom alias like gpt-4&v=2) would produce ...?model=gpt-4&v=2, causing the metering server to parse two parameters (model=gpt-4 and v=2) and check the balance against a truncated model name.

customer and feature are in path segments where &/= are legal, so they can keep using PATH_SEGMENT. Only the query-string value needs the additional escaping:

const QUERY_VALUE: &AsciiSet = &PATH_SEGMENT.add(b'&').add(b'=');

Then in build_balance_url:

let model = utf8_percent_encode(model, QUERY_VALUE);

Standard model names (gpt-4, claude-sonnet-4, llama-3.1-8b) are unaffected, but custom model aliases or model names sourced from untrusted input could trigger an incorrect balance check.