Proxy request signing: transform= on @proxy rules, plugin-registered schemes, aws-sigv4 plugin - #1008
Open
theoephraim wants to merge 30 commits into
Open
Proxy request signing: transform= on @proxy rules, plugin-registered schemes, aws-sigv4 plugin#1008theoephraim wants to merge 30 commits into
theoephraim wants to merge 30 commits into
Conversation
Adds a transform= option to @Proxy rules (attached or detached): the proxy computes an HMAC signature (hmac-sha256 / hmac-sha512) over the final outbound request, after placeholder substitution and upstream identity verification, and writes it into configurable headers. The signing secret is consumed by the signer and never substituted; its placeholder appearing anywhere in a request fails closed. Covers Coinbase/FalconX-class venue auth and webhook HMAC schemes via a templated string-to-sign plus encoding/timestamp options. First phase of issue #997; AWS SigV4 re-signing lands as an additional scheme later.
…tion Restructure transform config validation around PROXY_TRANSFORM_SCHEME_SPECS (required/optional options per scheme) so adding a scheme (aws-sigv4, custom) means adding a spec entry, a ProxyRuleTransform union member, and a signer - without loosening validation for existing schemes. Unknown-option and missing-required errors are now scheme-aware, and the rule builder copies options generically from the spec.
Contributor
|
The changes in this PR will be included in the next version bump.
|
Contributor
📦 Bundle size
dist/ only; native binaries are versioned separately and not counted here. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
varlock-website | 4cf5cdc | Commit Preview URL Branch Preview URL |
Sep 01 2026, 03:57 AM |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
A schema-valid credential-role collision can send the raw signing secret upstream. The audit path can also record a transform failure as both allowed and blocked.
Reviewed changes across the parser, resolved proxy graph, request runtime, audit model, tests, release entry, and user documentation.
- Transform schema and graph: Added per-scheme HMAC option validation, attached and detached secret resolution, managed-item roles, and runtime rule serialization.
- Signing runtime: Added templated HMAC SHA-256/SHA-512 signing after substitution and TLS identity verification, with approval gating and fail-closed handling.
- Audit and policy: Added transform-aware rule descriptions,
blocked-transform, andsignedWithactivity fields. - Coverage and documentation: Added unit and TLS integration tests plus reference and guide documentation for request signing.
azure/gpt-5.6-sol ｜ 𝕏
Re-signs AWS SDK requests made with placeholder credentials: the proxy parses region and service from the inbound Credential scope (no region/service config; one rule covers every AWS service), strips the placeholder signature headers, and re-signs with the real keys via @smithy/signature-v4 (node:crypto sha256 adapter, S3 path-encoding rules included). Supports session tokens, optional allowedRegions/allowedServices gates, and preserves an UNSIGNED-PAYLOAD sentinel. Pre-signed URLs (query-signed) fail closed with a distinct message. Signature correctness is pinned by an independent spec-derived vector test plus an e2e replay check over the exact bytes the upstream received.
theoephraim
changed the title
Contributor
There was a problem hiding this comment.
Caution
The SigV4 delta adds a credential-role collision that can expose the signing secret and rewrites unsupported S3 streaming bodies into requests AWS rejects.
Reviewed changes since the prior Pullfrog review focused on the new AWS SigV4 transform and its integration with proxy rule resolution and request forwarding.
- Added AWS SigV4 re-signing: Parsed region and service from placeholder-signed requests, applied optional allowlists, and re-signed outbound headers with real credentials.
- Added temporary-credential handling: Managed and substituted access key IDs and optional session tokens before generating fresh SigV4 headers.
- Added AWS coverage and documentation: Added independent signature vectors, TLS integration tests, S3 payload handling, validation tests, and user guidance.
azure/gpt-5.6-sol ｜ 𝕏
varlock
@varlock/nextjs-integration
@varlock/native-helper-darwin
@varlock/native-helper-linux-arm64
@varlock/native-helper-linux-x64
@varlock/native-helper-win32-x64
@varlock/aws-sigv4-plugin
commit: |
…4 to @varlock/aws-sigv4-plugin
Transforms become a plugin-extensible seam. Core keeps the zero-dependency
hmac schemes; provider-specific schemes live in plugins that carry their own
deps. Scheme specs gain typed options (string/headerName/template/stringList/
enum) with declared item roles (consumed vs wire), so validation, placeholder
management, substitution scoping, and runtime credential resolution are all
driven by one declaration. Plugins register schemes via
registerProxyTransformScheme; the graph registry flows to the proxy runtime
(and through reload), and plugin identity joins the proxy schema fingerprint.
Also applies the PR review findings:
- binary bodies pass through byte-exact when no placeholder is present (no
utf8 mangling under a valid signature)
- a consumed signing secret stays substitutable where another rule injects it
(dual use), instead of being blocked on every host
- an approval-gated transform bypassed by a more specific allow rule fails
closed instead of forwarding the request unsigned
- the allow/signedWith audit entry is recorded only after signing succeeds
(no contradictory allow+blocked pairs)
- transform header targets reject framing/identity headers (content-length,
host, cookie, ...)
- streaming SigV4 payloads (STREAMING-*) fail closed with a pointer at the
SDK setting instead of mis-signing; conflict detection is key/list-order
insensitive; {timestamp} templates require a timestampHeader
- varlock proxy rules renders transforms and labels signing secrets honestly;
live and audit log lines show the signing scheme
The smithy deps move out of the root package.json into the plugin (via the
catalog).
theoephraim
changed the title
Contributor
There was a problem hiding this comment.
Important
Trusted reload can mix policy generations for in-flight signing requests.
Reviewed changes since the prior Pullfrog review covered the plugin-extensible transform registry, the AWS SigV4 package extraction, runtime reload propagation, and the fixes for earlier audit and streaming-payload findings.
- Generalized transform schemes: Added typed plugin registrations that drive option validation, credential roles, managed-item selection, and signer dispatch.
- Extracted AWS signing: Moved SigV4 implementation, dependencies, tests, and documentation into
@varlock/aws-sigv4-plugin. - Hardened request processing: Preserved binary bodies, rejected unsupported streaming payloads, made transform conflict comparison order-insensitive, and delayed success audit events until signing completes.
- Propagated runtime registries: Passed graph-loaded signer registries through startup and trusted reload and added plugin identity to the schema fingerprint.
azure/gpt-5.6-sol ｜ 𝕏
…ugins The aws-sigv4 scheme works unchanged with any SigV4-authenticated service (Cloudflare R2, MinIO, Backblaze B2, DynamoDB-compatible endpoints): the client is still an AWS SDK pointed at a custom endpoint and the rule's domain decides where it applies. Note this on the plugin page (with an R2 example), the rules guide, and the npm README. The plugins overview also gains a 'Credential proxy' category, since this is the first plugin that extends varlock beyond loading external secrets.
Contributor
There was a problem hiding this comment.
ℹ️ No critical issues in the new commit; one documentation correction is inline.
Reviewed changes since the prior Pullfrog review covered the expanded third-party SigV4 compatibility guidance and the new credential-proxy plugin categorization.
- Expanded SigV4 guidance: Added S3-compatible and DynamoDB-compatible service examples to the plugin README and website documentation.
- Reorganized plugin documentation: Added a credential-proxy plugin category and linked the AWS SigV4 plugin from the overview and package reference.
azure/gpt-5.6-sol ｜ 𝕏
Basic auth defeats placeholder substitution on its own: the child sends Authorization: Basic base64(user:placeholder), and the encoded placeholder never appears as a substring the proxy could swap. The http-basic scheme has the proxy compose the header itself from the real secret (static username or a wire-role usernameItem; colon usernames rejected per RFC 7617). Zero deps, so it joins the hmac schemes in core.
Contributor
There was a problem hiding this comment.
Caution
The new HTTP Basic transform can return a decodable managed password to the child through reflected request headers.
Reviewed changes since the prior Pullfrog review covered the new built-in HTTP Basic credential transform.
- Added HTTP Basic composition: Added schema options for static or managed usernames and generated
Authorization: Basic ...from the real password at the proxy boundary. - Added validation and coverage: Rejected conflicting username forms and colon-containing usernames, documented the scheme, and added unit, graph, and TLS integration tests.
azure/gpt-5.6-sol ｜ 𝕏
…rails The http-basic scheme's options become username= and password=. Schemes can now declare their own consumed-role option, replacing the generic secretKey name at the schema surface (rule data still canonicalizes to secretKey, so the runtime and signers are unchanged). username= takes a literal or a $REF (resolved to the item's value, since usernames are wire-visible config); secretIn="username" covers single-token APIs where the token is the userid with an empty password. Guardrails for credential-name options everywhere: they take the item's NAME, and a $ reference (which would resolve the real secret's value into rule data) is rejected statically for built-in schemes, with a resolve-time unknown-item backstop for plugin schemes that deliberately does not echo the offending value. A literal password fails the same check with guidance; a genuinely static password belongs in an item.
Contributor
There was a problem hiding this comment.
Important
The new item-name guard can still accept forbidden references and silently select a different credential.
Reviewed changes since the prior Pullfrog review covered the HTTP Basic schema revision and credential item-name validation.
- Refined HTTP Basic configuration: Replaced the generic consumed-secret option with
password=, allowed$-resolved username values, and addedsecretIn="username"for token-as-userid APIs. - Added item-name guardrails: Rejected dynamic item-role arguments for statically known built-in schemes and added a non-echoing existence check after transform resolution.
- Expanded coverage and guidance: Added graph and signer cases for detached passwords, invalid references, username resolution, and the new token placement mode.
azure/gpt-5.6-sol ｜ 𝕏
…without resolving Transform options that point at other items are now written as references (password=$REGISTRY_PASSWORD, keyId=$AWS_ACCESS_KEY_ID), consistent with the rest of the schema language, instead of name strings. The parser expands $X to ref(X); the @Proxy load pass swaps those ref resolvers for static $NAME markers BEFORE resolution runs, so a referenced credential's value never resolves into rule data or anything serialized from it. Rule data canonicalizes to bare item names; the proxy resolves real values only at signing time. Credential options require the reference form (a literal is rejected with a pointed error, statically for built-in schemes and at resolve time for plugin schemes). http-basic's username accepts a literal or a reference; a referenced username becomes a wire-role item like any other.
With the consumed secret in the userid position, the password side is ordinary config, so password= may then be a plain literal (GitHub-style TOKEN:x-oauth-basic), defaulting to empty. In the default mode a literal password is still rejected. Generically: a consumed option canonicalizes to secretKey only when written as a $ reference; a literal on a literal-allowed consumed option passes through as a scheme option and the attached item supplies the secret.
Contributor
There was a problem hiding this comment.
Important
The new credential-reference capture also intercepts non-credential transform references, breaking dynamic transform options. Several documentation examples still use the now-invalid quoted item-name syntax.
Reviewed changes since the prior Pullfrog review covered the new $ITEM syntax and pre-resolution capture for transform credential options.
- Changed credential references: Required item-role options to use
$ITEMsyntax and preserved those references without resolving credential values into rule data. - Extended runtime resolution: Added marker-aware role discovery and proxy-time resolution, including managed HTTP Basic usernames.
- Updated validation and guidance: Added literal rejection and missing-item cases, and revised most request-signing examples for the new syntax.
azure/gpt-5.6-sol ｜ 𝕏
username=$ITEM is for non-secret usernames (wire-role, substitutable); a secret userid belongs in secretIn=username so it gets consumed-secret protections. Prevents the working-but-weaker footgun of passing a token as a username reference.
Contributor
There was a problem hiding this comment.
Important
A quoted literal password beginning with $ can be misinterpreted as an item reference and select a different managed credential.
Reviewed changes since the prior Pullfrog review covered the fixed-password extension for HTTP Basic token-as-userid authentication and its follow-up documentation clarification.
- Added fixed Basic passwords: Allowed
secretIn="username"transforms to pair the consumed userid secret with a literal password, including GitHub-styleTOKEN:x-oauth-basiccredentials. - Clarified username handling: Documented that
$ITEMusernames are wire-visible and that secret userids must usesecretIn="username".
azure/gpt-5.6-sol ｜ 𝕏
…ples with a test The item- and root-decorator reference rows still described the pre-plugin, pre-reference surface (no http-basic, secretKey="ITEM_NAME" strings), and two guide examples still passed credential items as name strings. Adds a test that loads the documented examples through the real graph and asserts no credential value reaches rule data, so the docs cannot drift from the syntax again.
Contributor
There was a problem hiding this comment.
ℹ️ The documentation corrections are sound; one minor test-coverage gap is inline.
Reviewed changes since the prior Pullfrog review covered the corrected transform references and the new documentation-example regression test.
- Corrected credential examples: Replaced quoted HMAC, SigV4, and detached signing item names with the required
$ITEMreference syntax. - Added example validation: Added graph-loading coverage for the documented HMAC and HTTP Basic configurations and checked that credential values stay out of rule data.
azure/gpt-5.6-sol ｜ 𝕏
…mmetric secretIn existed only because password was declared the single consumed option and username a wire-role one, so there was no way to say the secret is the userid without a mode switch. That forced an option whose type depended on another option's value and a cross-field validate hook. Both sides are now symmetric: each takes a literal or a $ITEM reference, and any reference is a consumed credential. On an attached rule the decorated item fills whichever side is unset, defaulting to the userid when neither is given (the curl -u "token:" convention that single-credential Basic APIs use). This also gains the both-sides-secret case (Twilio-style SID:AuthToken), which the old model could only express by giving the SID weaker wire-role treatment. Schemes place the attached item via a placeAttachedItem hook, so a scheme with more than one credential position decides for itself; single-secret schemes (hmac, aws-sigv4) keep the default fill. The secretKey canonicalization that existed only to rename http-basic's consumed option is gone.
Contributor
There was a problem hiding this comment.
ℹ️ No new issues in this incremental change; existing review threads remain open.
Reviewed changes since the prior Pullfrog review covered the new role-discovery regression test.
- Added credential-role coverage: Exercised
getTransformRoleKeysfor both HTTP Basic credential slots, the attached-item default, and HMAC consumed versus wire roles used byproxy rulesoutput.
azure/gpt-5.6-sol ｜ 𝕏
Credential references (findings: provenance, over-broad capture):
- stop rewriting ref resolvers at decorator load, which broke every other $REF
inside transform{} (signatureHeader=$HEADER became a literal, allowedRegions
=$REGIONS became a marker) and made a quoted literal starting with $
indistinguishable from a reference
- credential options now take their item NAME from the UNRESOLVED decorator
args and carry it as a typed { itemRef } marker, so a value still never
reaches rule data while every non-credential option resolves normally and a
literal stays a literal
Collisions:
- reject two credential roles referencing the same item (keyId/sessionToken
equal to secretKey would send the signing secret upstream)
- reject two options writing the same destination header (a key or timestamp
write silently overwriting the signature)
Runtime:
- snapshot rules, managed items, egress mode, and the scheme registry at
request entry, so reconfigure() cannot swap policy under an in-flight request
- signers can declare reversible output via scrubFromResponse; http-basic
declares its base64 token so a reflected Authorization header cannot hand the
child a decodable credential (consumed secrets are not in hostItems, so
response scrubbing did not know them)
Docs:
- qualify the sigv4 S3-compatibility claim (SDK checksum defaults produce
aws-chunked uploads the proxy cannot re-sign) and file GCS under S3-compatible
- the docs regression test now reads the published .mdx snippets instead of
duplicating them
Contributor
There was a problem hiding this comment.
Important
The credential-collision fix overreaches and rejects valid same-role configurations.
Reviewed changes since the prior Pullfrog review covered the consolidated fixes for transform references, validation, response scrubbing, reload consistency, and documentation coverage.
- Separated credential references: Replaced ambiguous
$NAMEstrings with{ itemRef }objects while preserving normal resolution for non-credential options and literal$-prefixed values. - Added transform collision guards: Rejected duplicate credential references and case-insensitive destination-header collisions.
- Protected Basic credentials: Added signer-declared response scrub values and an end-to-end reflected-Authorization regression test.
- Snapshotted runtime policy: Captured rules, managed items, egress mode, schemes, and consumed keys once per request across reloads.
- Tied tests to published docs: Loaded transform examples directly from MDX and corrected SigV4 compatibility guidance.
azure/gpt-5.6-sol ｜ 𝕏
…credential
A literal credential side accepts interpolation (username="acct-\${ID}"),
which resolves normally and is useful for ordinary config. The same syntax on a
secret silently embedded the real value in the rule, bypassing the reference
mechanism. Credential options now reject interpolating any item varlock treats
as sensitive, pointing at the two remedies: pass it by reference, or mark a
non-secret item @sensitive=false.
Contributor
There was a problem hiding this comment.
ℹ️ No new issues in this incremental change; existing review threads remain open.
Reviewed changes since the prior Pullfrog review covered the new sensitive-interpolation guard for transform credentials.
- Protected credential options: Traversed nested resolver arguments and rejected sensitive item interpolation before a resolved value can enter transform rule data, while retaining non-sensitive config interpolation.
- Documented interpolation semantics: Explained that literal credential sides may interpolate only items explicitly marked non-sensitive and that credentials must use direct
$ITEMreferences. - Added graph coverage: Verified allowed non-sensitive interpolation and rejection of a sensitive value nested in an HTTP Basic credential option.
azure/gpt-5.6-sol ｜ 𝕏
…g time
Interpolation was rejected on credential options because a reference was
represented as a single item name, so "pre-\${SECRET}" had nowhere to live
except as a resolved string in rule data. A credential value is now an ordered
list of literal parts and item references, composed by the proxy when it signs.
The restriction disappears: username="acct-\${TENANT}" and
password="\${TOKEN}-suffix" both work, and a referenced value stays out of
rule data exactly like a bare reference.
Every item referenced from a credential option, alone or interpolated, is a
consumed credential: managed, excluded from substitution, leak-guarded, and
scrubbed from responses. That covers usernames holding sensitive data, which
the Basic token carries on the wire.
Contributor
There was a problem hiding this comment.
Important
The new interpolation path still allows nested resolver forms to serialize a real credential into proxy rule data.
Reviewed changes since the prior Pullfrog review covered sign-time composition for interpolated transform credentials.
- Added credential templates: Preserved ordered literal and
$ITEMparts in transform rule data and composed their resolved values only when signing. - Expanded role handling: Collected every referenced item from templates for managed-item selection, collision checks, and runtime credential resolution.
- Added focused coverage and docs: Tested template composition and documented interpolation on HTTP Basic credential sides.
azure/gpt-5.6-sol ｜ 𝕏
Reverts the interpolation machinery in favor of a smaller rule: a credential option is always $ITEM. There are no literals in credential positions, so there is nothing to interpolate, no template to reconstruct, and no sensitivity judgment to make about an inline value. Anything computed (a fixed prefix, a constant like GitHub's x-oauth-basic) is composed in the item and referenced, which is how values work everywhere else in a schema. Deletes the parts/template type, the resolver-tree flattener, the sign-time composition, the literalAllowed flag, and the literal-vs-reference branching in the signer. Also settles the sensitive-username question by construction: both Basic sides are references, so a username holding sensitive data gets the same managed, non-substitutable, leak-guarded treatment as the password. Motivating check: rules are never serialized (the session record stores the fingerprint, placeholder overrides, and omitted keys, not rules), so keeping values out of rule data is hygiene inside the process that already holds them. References earn their place for identity, not secrecy: the item name is what drives placeholder management, leak guarding, and substitution scoping.
Contributor
There was a problem hiding this comment.
ℹ️ No new issues in this incremental change; the existing same-role credential reuse thread remains open.
Reviewed changes since the prior Pullfrog review covered the direct-reference-only credential model.
- Restricted credential options: Required every item-role option to use a direct
$ITEMreference, rejecting inline literals, interpolation, and nested resolver expressions before resolved values can enter rule data. - Moved credential composition: Kept fixed and computed values in referenced config items, with the proxy resolving only the final item value at signing time.
- Simplified runtime handling: Removed credential-template composition and retained single item references across role discovery, runtime resolution, signer types, tests, docs, and release notes.
azure/gpt-5.6-sol ｜ 𝕏
… only derived forms Response scrubbing was scoped to the items substituted into the request, so a consumed signing credential, or a secret bound for a different route, could be reflected back to the child untouched. It now covers every sensitive value the proxy holds, which is the rule that should have applied all along. http-basic's signer therefore declares only what the runtime cannot compute: the base64 token. It previously declared the raw credentials too, which over-reached in the other direction, registering an ordinary non-secret username as a scrub target and rewriting it out of legitimate response data. Managed items now carry their sensitivity so the runtime can tell the difference; a missing flag is treated as sensitive.
Contributor
There was a problem hiding this comment.
Important
Global response scrubbing now risks corrupting unrelated proxied traffic when a sensitive value is short or common.
Reviewed changes since the prior Pullfrog review covered the response-scrubbing expansion in 77ac1fee.
- Expanded sensitive-value scrubbing: Propagated item sensitivity into managed proxy items, scrubbed every sensitive managed value from responses, and limited signer-provided scrub values to derived reversible forms.
- Added end-to-end coverage: Verified that raw secrets, an unrelated-route secret, and the generated Basic token are removed while an explicitly non-sensitive username remains intact.
azure/gpt-5.6-sol ｜ 𝕏
'The signing secret never travels' held for the hmac schemes and aws-sigv4, which send only a derived signature, but not for http-basic: Basic auth carries the credentials themselves, base64-encoded. The guide, the `proxy rules` label, the leak-guard message, and the signer's own docstring all repeated that claim. They now state the guarantee that holds for every scheme (the agent never holds the credential, so it cannot produce a valid request) and, in the guide, call out the per-scheme difference in whether the value reaches the API.
Leads with the primitive and what it does, then names the built-in schemes and the plugin extension point, instead of enumerating implementation details.
Brings the branch up to date (30 commits) and adopts the current conventions. Reconciled with #1046, which removed maxOccurrences and reworked substitution to a per-surface, one-per-target model: - rules keep transform=; maxOccurrences is gone, and the migration error from main is the single source of guidance for it - substitution uses main's per-surface substitutePlaceholdersInSurface, with the branch's binary-body fix preserved by keeping the ORIGINAL bytes unless substitution actually changed the body (a utf8 round-trip would corrupt a binary payload, and a signature over the mangled bytes would legitimize it) - the allow audit entry keeps the branch's ordering (recorded only after signing succeeds) and carries main's skippedPlaceholders - the request log line shows both sign: and skipped: The new plugin follows current conventions: tsdown instead of tsup, and the exports-only entrypoint shape (no main/types) that the migration standardized, which publint now enforces at build time.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
varlock-docs-mcp | 1e330d3 | Aug 31 2026, 10:34 PM |
Contributor
There was a problem hiding this comment.
ℹ️ No new issues in the incremental changes; the existing same-role credential reuse and global response-scrubbing threads remain open.
Reviewed changes since the prior Pullfrog review covered the merge from main and its interactions with request transforms.
- Adopted scoped substitution: Kept placeholders outside configured request surfaces inert, assigned one substitution budget per named target, and split path and query rewriting while preserving the final bytes passed to signers.
- Preserved binary request bodies: Retained original body buffers when substitution made no textual change, including through the transform pipeline.
- Migrated the plugin build: Switched
@varlock/aws-sigv4-pluginfromtsuptotsdown, added package validation, and retained bundled Smithy dependencies.
azure/gpt-5.6-sol ｜ 𝕏
Basic auth is not a signature, and future schemes may not sign either. The generic layer now says transform: ProxyTransformFn/Input/Result, the scheme def's sign() is apply(), and the audit field is transformedWith. Signing vocabulary stays where it is accurate (hmac, aws-sigv4). Also pin the smithy deps directly in the aws-sigv4 plugin instead of the root catalog, matching how every other plugin handles a single-consumer dep.
Two review findings:
- The credential collision check rejected any two options naming the same
item, but only a consumed/wire pair is an exposure. http-basic consumes both
sides, so username=$X password=$X is merely redundant and now loads; the
error for a real cross-role collision names which side goes on the wire.
- Response scrubbing covered every sensitive item on every MITM'd request, so
a short value bound to another route ("ok") rewrote ordinary text in an
unrelated response. Values that can reach this upstream are still scrubbed
at any length; others must clear a length floor.
A response can only legitimately reflect what that upstream was given, so
scrubbing now puts back placeholders for the rule's own items plus the active
transform's credentials, rather than scanning for every sensitive value the
proxy holds. Scanning globally meant a short value bound to another route
("ok") rewrote ordinary text in unrelated responses, and a cross-route match
is indistinguishable from coincidence anyway.
The child is never meant to hold any managed secret, so learning one from an unrelated upstream is exactly as damaging as learning it from the route it belongs to. Reflected placeholders are inert and pass through untouched. Accepts that a pathologically short credential could rewrite ordinary response text: over-scrubbing corrupts a payload visibly, while under-scrubbing hands the child a live secret. Managed items are only ever items a @Proxy rule references, so the pool is credentials rather than arbitrary env values.
Contributor
There was a problem hiding this comment.
ℹ️ No critical issues in the new commits; one plugin integration fixture regression is inline.
Reviewed changes since the prior Pullfrog review covered the scheme-neutral API rename, credential collision refinement, and response-scrubbing revisions.
- Renamed transform contracts: Replaced signer-specific public types, callbacks, audit fields, logs, and documentation with scheme-neutral transform terminology.
- Allowed same-role credential reuse: Restricted duplicate-item rejection to consumed-to-wire collisions and added HTTP Basic coverage for one item in both consumed positions.
- Revisited response scrubbing: Temporarily scoped response values by route, then restored global sensitive-value scrubbing with explicit placeholder and cross-route coverage.
- Adjusted plugin packaging: Moved the Smithy versions out of the root catalog and normalized the package repository URL.
azure/gpt-5.6-sol ｜ 𝕏
…runtime The rename swept .ts and .mdx, so the .js plugin fixture kept registering sign() and would have produced a scheme whose apply is undefined, 502ing any request that used it. Nothing caught this: the existing runtime tests build a scheme def by hand, and the loader test only inspected rule metadata. Adds an integration test covering schema -> @plugin -> registry -> runtime -> outbound request, asserting the header the fixture's own code writes.
Contributor
There was a problem hiding this comment.
ℹ️ No new issues in the incremental change; the existing global response-scrubbing thread remains open.
Reviewed changes since the prior Pullfrog review covered the plugin transform fixture and its new end-to-end regression test.
- Migrated the plugin fixture: Replaced the stale
signcallback with the scheme-neutralapplycontract. - Exercised the public plugin seam: Loaded the fixture through
@plugin, propagated its registered scheme into the proxy runtime, and verified the outbound header uses both resolved credentials.
azure/gpt-5.6-sol ｜ 𝕏
Scrubbing is substring replacement with no token boundary, so it cannot tell a reflected credential from ordinary text that matches it. Two changes: - Scrub only the values a matching rule sends to that upstream, so a secret bound to another route can't rewrite content here. - Warn at proxy start when a proxied secret is short enough to double as ordinary content (an org slug, an account id, a dev password), since scoping alone doesn't help on the value's own route. @sensitive=false is the fix when it isn't really a secret.
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review covered the response-scrubbing containment fix and its user-facing warning.
- Scoped response scrubbing: Limited sensitive-value replacement to items associated with matching rules, leaving cross-route values untouched.
- Added short-secret guidance: Warned at proxy startup when a managed sensitive value is short enough to collide with ordinary response content and documented the limitation.
- Expanded regression coverage: Added focused helper tests and end-to-end assertions for cross-route response integrity.
azure/gpt-5.6-sol ｜ 𝕏
…text Redaction is substring replacement with no token boundary, so a short sensitive value gets rewritten wherever it appears, corrupting console output and proxied response bodies. Nothing guarded against this: every sensitive value was registered for redaction regardless of length. Adds an advisory validation warning surfaced by varlock load, and moves the threshold into a shared module the proxy uses too. @sensitive=false is the fix when the value is not really a secret; short-by-nature secrets (an OTP, a PIN) get the collision heads-up without a remedy.
Contributor
There was a problem hiding this comment.
Important
The new general short-sensitive-value warning is not surfaced during successful CLI runs.
Reviewed changes since the prior Pullfrog review covered the expansion of short-sensitive-value diagnostics beyond proxy startup.
- Centralized collision detection: Moved the short-sensitive-value threshold and predicate into a shared module used by proxy startup and config validation.
- Added item validation warnings: Attached an advisory warning to every resolved sensitive value shorter than 12 characters and updated affected tests.
- Documented general behavior: Added guidance about substring-redaction collisions and the
@sensitive=falseopt-out.
azure/gpt-5.6-sol ｜ 𝕏
It applies to every varlock user, not just proxy ones: log redaction has the same substring-collision problem. Keeping it here mixed an env-graph validation change (and a validationState flip for short sensitive items) into a proxy feature review.
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since the prior Pullfrog review covered the removal of the short-sensitive-value warning work from this PR.
- Removed the warning implementation: Removed the general config validation warning, proxy startup warning, shared threshold helper, and related test changes so they can land separately.
- Restored focused documentation: Removed the warning-specific guidance while retaining the response-scrubbing route-scoping caveat introduced by this PR.
azure/gpt-5.6-sol ｜ 𝕏
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #997: request transforms at the credential proxy, as a plugin-extensible seam. Signing is the motivating case, but the primitive is any scheme that computes what a request carries, so the generic layer is named for transforms and only the schemes that actually sign talk about signing.
What
Core: the
transform=seam and its built-in schemes.@proxyrules (attached or detached) gain atransform=option: the proxy computes the credential for the final outbound request, after placeholder substitution and upstream identity verification, from a secret the agent never holds. Core ships three zero-dependency schemes.hmac-sha256/hmac-sha512sign a templated string over{timestamp} {method} {path} {pathWithQuery} {query} {host} {body}, with output/key encodings and timestamp formats, covering Coinbase/FalconX-class venue auth and webhook HMAC schemes without per-venue code.http-basiccomposes theAuthorization: Basicheader itself, which substitution cannot do at all (base64 hides the placeholder from the swap). Itsusernameandpasswordoptions are symmetric, and each references the item holding that side's value: either side may be the credential, so one rule covers a secret password, a token-as-userid (curl -u "token:"), a token plus a fixed password (GitHub'sTOKEN:x-oauth-basic), and both sides secret (Twilio'sSID:AuthToken). On an attached rule the decorated item fills whichever side is unset, the userid when neither is given.Credential items are always passed as
$ITEMreferences, never inline values, consistent with the rest of the schema language. The item NAME is read from the unresolved decorator args, so a credential's value never resolves into rule data; the proxy resolves it at signing time. That name is also what drives placeholder management, leak guarding, and substitution scoping, and it means a username holding sensitive data gets the same protections as a password. Anything computed (a fixed prefix, a constant) is composed in the item and referenced.Plugin-registered schemes. Schemes are declared as typed option specs (string / headerName / template / stringList / enum, plus item roles:
consumed= the signing secret, never on the wire;wire= key ids and session tokens that travel and substitute normally). One declaration drives validation, placeholder management, substitution scoping, and runtime credential resolution. Plugins add schemes viaregisterProxyTransformScheme(alongsideregisterResolverFunctionetc.); the graph's registry flows to the proxy runtime and through reload, and plugin identity joins the proxy schema fingerprint so a reload that swaps transform code is surfaced by the same gating that watches schema edits.@varlock/aws-sigv4-plugin(new package). AWS SigV4 re-signing as the first plugin-provided scheme, keeping the@smithy/*deps out of core. The agent's SDK signs with placeholder credentials; the proxy parses region/service from the inboundCredential=scope (no region/service config; one rule covers every AWS service), strips the placeholder signature, and re-signs with the real keys via@smithy/signature-v4. Session tokens supported; optionalallowedRegions/allowedServicesgates;UNSIGNED-PAYLOADpreserved; pre-signed URLs andSTREAMING-*aws-chunked payloads fail closed with distinct messages.Why
Substitution can't cover APIs where the secret never travels and every request instead carries a signature computed with it. Computing the credential at the wire is a stronger boundary than substitution: the child can't produce a valid request even in principle, since it never holds the underlying secret. Provider-specific schemes as plugins keep core dependency-free and give custom/venue schemes (including local, unpublished plugins) the same validated path as built-ins.
Key semantics
blocked-transform). An item another rule legitimately injects stays substitutable there (dual use).content-length,host,cookie, ...);{timestamp}templates require atimestampHeader; equivalent configs from multiple rules merge (order-insensitive) while genuinely different ones fail closed.transformedWithaudit entry is recorded only after the transform succeeds.http-basicadds its base64 token on top, since only the scheme can produce that form. Reflected placeholders are inert and pass through untouched. Scrubbing is substring replacement and can't tell a reflected credential from matching prose, sovarlock proxynow warns when a proxied secret is short enough to double as ordinary content (an org slug, an account id).varlock proxy rulesshows transform rules and labels transform credentials; live and audit log lines show the scheme.Testing
Core: HMAC vectors, spec-driven validation, and MITM e2e over the scheme-registry seam (fixture scheme: credential resolution by role, set/remove header application, single audit entry, dual-use, approval fail-closed, order-insensitive conflict detection, byte-exact binary passthrough), plus a fixture plugin through the real plugin loader. Plugin: independent spec-derived SigV4 vector tests (node:crypto only), streaming/presigned fail-closed cases, and a full-pipeline MITM e2e where the received request's signature is re-derived from the SigV4 spec by hand and matched byte-for-byte.