Skip to content

Proxy request signing: transform= on @proxy rules, plugin-registered schemes, aws-sigv4 plugin - #1008

Open
theoephraim wants to merge 15 commits into
mainfrom
proxy-request-signing
Open

Proxy request signing: transform= on @proxy rules, plugin-registered schemes, aws-sigv4 plugin#1008
theoephraim wants to merge 15 commits into
mainfrom
proxy-request-signing

Conversation

@theoephraim

@theoephraim theoephraim commented Aug 14, 2026

Copy link
Copy Markdown
Member

Implements #997: request signing at the credential proxy, as a plugin-extensible seam.

What

Core: the transform= seam and its built-in schemes. @proxy rules (attached or detached) gain a transform= option: the proxy signs the final outbound request, after placeholder substitution and upstream identity verification, with a secret the agent never holds. Core ships three zero-dependency schemes. hmac-sha256 / hmac-sha512 sign 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-basic composes the Authorization: Basic header itself, which substitution cannot do at all (base64 hides the placeholder from the swap). Its username and password options are symmetric: each takes a literal or a $ITEM reference, any reference is a consumed credential, and on an attached rule the decorated item fills whichever side is unset (the userid when neither is given, the curl -u "token:" convention). That covers a secret password, a token-as-userid, a token plus a fixed password (GitHub's TOKEN:x-oauth-basic), and both sides secret (Twilio's SID:AuthToken) with no mode switch.

Credential items are passed as $ITEM references, consistent with the rest of the schema language. Inside a transform, those references are captured as markers before resolution runs, so a referenced credential's value never resolves into rule data or anything serialized from it; the proxy resolves real values only at signing time. Credential options require the reference form (a literal is rejected), while username also accepts a plain literal, since a fixed username is ordinary config.

# @proxy(domain="api.exchange.com", transform={
#   scheme="hmac-sha256",
#   stringToSign="{timestamp}{method}{pathWithQuery}{body}",
#   signatureHeader="X-ACCESS-SIGN", timestampHeader="X-ACCESS-TIMESTAMP",
#   keyId="EXCHANGE_API_KEY", keyHeader="X-ACCESS-KEY", encoding="hex",
# })
EXCHANGE_API_SECRET=somePlugin()

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 via registerProxyTransformScheme (alongside registerResolverFunction etc.); 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 signer 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 inbound Credential= 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; optional allowedRegions/allowedServices gates; UNSIGNED-PAYLOAD preserved; pre-signed URLs and STREAMING-* aws-chunked payloads fail closed with distinct messages.

# @plugin(@varlock/aws-sigv4-plugin)
# ---
# @proxy(domain="*.amazonaws.com", transform={
#   scheme="aws-sigv4", keyId="AWS_ACCESS_KEY_ID", allowedServices=[bedrock, s3],
# })
AWS_SECRET_ACCESS_KEY=somePlugin()

Why

Substitution can't cover APIs where the secret never travels and every request instead carries a signature computed with it. Signing at the wire is a stronger boundary than substitution: the child can't produce a valid signature even in principle, since it never holds the key. 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

  • The signing secret is consumed by the signer, never substituted; its placeholder in a request it is not injectable into fails closed (blocked-transform). An item another rule legitimately injects stays substitutable there (dual use).
  • Binary bodies pass through byte-exact when no placeholder is present; SigV4 hashes exact outbound bytes.
  • An approval-gated transform bypassed by a more specific allow rule fails closed rather than forwarding the request unsigned; approval-gated rules sign only after approval passes.
  • Transform header targets reject framing/identity headers (content-length, host, cookie, ...); {timestamp} templates require a timestampHeader; equivalent configs from multiple rules merge (order-insensitive) while genuinely different ones fail closed.
  • Child-sent signature/timestamp headers are overwritten; cleartext connections are refused; the allow/signedWith audit entry is recorded only after signing succeeds.
  • varlock proxy rules shows signing rules and labels signing secrets as consumed-never-sent; 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.

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.
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

bumpy-frog

The changes in this PR will be included in the next version bump.

minor Minor releases

  • varlock 1.16.1 → 1.17.0

Bump files in this PR

Click here if you want to add another bump file to this PR


This comment is maintained by bumpy.

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle size

⚠️ grows the bundle by 221.1 KB (+4.4%)

Metric main This PR Δ
Total dist 5031.8 KB 5252.8 KB +221.1 KB (+4.4%)
JS 1723.0 KB 1812.6 KB +89.6 KB (+5.2%)
Sourcemaps 3232.2 KB 3359.6 KB +127.4 KB (+3.9%)
Type defs 76.6 KB 80.6 KB +4.0 KB (+5.2%)

dist/ only; native binaries are versioned separately and not counted here.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 14, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
varlock-website 1cdbfb1 Commit Preview URL

Branch Preview URL
Aug 31 2026, 12:28 AM

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, and signedWith activity fields.
  • Coverage and documentation: Added unit and TLS integration tests plus reference and guide documentation for request signing.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

Comment thread packages/varlock/src/proxy/types.ts Outdated
Comment thread packages/varlock/src/proxy/runtime-proxy.ts Outdated
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 theoephraim changed the title Proxy request signing: transform= on @proxy rules (HMAC) Proxy request signing: transform= on @proxy rules (HMAC + AWS SigV4) Aug 14, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

Comment thread packages/varlock/src/proxy/types.ts Outdated
?? checkHeaderName('timestampHeader')
?? checkString('secretKey')
?? checkString('keyId')
?? checkString('sessionToken')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new sessionToken role can name the same item as secretKey, so the runtime resolves both roles to the signing secret and Smithy sends its real value as X-Amz-Security-Token. This extends the credential-role collision into a new direct secret-exposure path.

Technical details
# Reject collisions between consumed and wire-visible SigV4 credentials

## Affected sites
- `packages/varlock/src/proxy/types.ts:308` - validates `sessionToken` as a string but does not compare it with `secretKey`.
- `packages/varlock/src/env-graph/lib/env-graph.ts:1354` - adds the session-token item to the wire-visible role set, then filters it out when it equals the signing secret.
- `packages/varlock/src/proxy/runtime-proxy.ts:1311` - resolves both roles from the same managed item and passes the real secret as the session token.

## Required outcome
- A consumed signing-secret item must not be accepted as a wire-visible access key ID or session token.
- Add validation coverage for `sessionToken === secretKey`, including the attached-rule default for `secretKey`.

Comment thread packages/varlock/src/proxy/aws-sigv4-transform.ts Outdated
@pkg-pr-new

pkg-pr-new Bot commented Aug 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/varlock@1008

commit: d035397

…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 theoephraim changed the title Proxy request signing: transform= on @proxy rules (HMAC + AWS SigV4) Proxy request signing: transform= on @proxy rules, plugin-registered schemes, aws-sigv4 plugin Aug 26, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

// garbage signature); ours overwrite them, mirroring how iron-style proxies
// strip inbound placeholder signatures before re-signing.
if (activeTransform) {
const schemeDef = transformSchemes[activeTransform.scheme];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An in-flight request captures activeTransform before body reading, approval, and upstream verification, but this lookup and the credential resolution below read mutable bindings that reconfigure() can replace meanwhile. A reload can therefore run an old transform config with a new signer or credential set, violating the documented per-request snapshot behavior.

Technical details
# Snapshot transform runtime state per request

## Affected sites
- `packages/varlock/src/proxy/runtime-proxy.ts:1373` reads the mutable scheme registry after multiple async boundaries.
- `packages/varlock/src/proxy/runtime-proxy.ts:1386` resolves credentials from the mutable managed-item set.
- `packages/varlock/src/proxy/runtime-proxy.ts:1740-1744` replaces rules, credentials, schemes, and consumed keys independently during reload.

## Required outcome
- Each request must use one immutable generation of rules, managed items, egress mode, transform schemes, and consumed keys from policy evaluation through signing and forwarding.
- Add a reload test that pauses an in-flight transformed request, reconfigures the runtime, and verifies that the old request finishes with its original signer and credentials while the next request uses the new generation.

…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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ 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.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

Comment thread packages/varlock-website/src/content/docs/plugins/aws-sigv4.mdx Outdated
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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

Comment thread packages/varlock/src/proxy/request-transform.ts Outdated
…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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 added secretIn="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.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

Comment thread packages/varlock/src/env-graph/lib/env-graph.ts
…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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 $ITEM syntax 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.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

// serialization of it. Scheme-agnostic on purpose (plugin schemes included);
// whether a given option may be a ref is validated against the scheme spec.
for (const [key, val] of Object.entries(inner)) {
if (val?.fnName !== 'ref') continue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rewrites every direct ref(...), including ordinary options that should still resolve normally. For example, signatureHeader=$HEADER now fails validation on the literal $HEADER, while allowedRegions=$REGIONS becomes a policy list containing the marker rather than the referenced value; capture must be limited to options that the selected scheme declares with itemRole while retaining support for plugin schemes.

Technical details
# Preserve normal resolution for non-credential transform options

## Affected sites
- `packages/varlock/src/env-graph/lib/decorators.ts:505-510` captures every direct reference before the scheme option spec is consulted.
- `packages/varlock/src/proxy/types.ts:465-496` consequently validates the `$NAME` marker as the final value for non-item option types.

## Required outcome
- Preserve pre-resolution provenance for item-role options without changing the normal resolved values of `scheme`, `headerName`, `template`, `stringList`, and `enum` options.
- Add coverage for a referenced non-item transform option, including a plugin-provided scheme.

Comment thread packages/varlock-website/src/content/docs/guides/proxy/rules.mdx Outdated
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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-style TOKEN:x-oauth-basic credentials.
  • Clarified username handling: Documented that $ITEM usernames are wire-visible and that secret userids must use secretIn="username".

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

// config, so it stays a scheme option and the attached item supplies the
// secret instead.
const rawConsumed = obj[consumedOption];
const consumedRef = _.isString(rawConsumed) && rawConsumed.startsWith('$')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quoted literal password beginning with $ is treated as an item reference here because quote/reference provenance has already been discarded. password="$fixed" can therefore select another managed item as the username secret, or fail as a missing item, instead of producing the requested literal Basic password.

Technical details
# Preserve literal password provenance

## Affected sites
- `packages/varlock/src/env-graph/lib/decorators.ts:499-509` converts actual references to plain `$NAME` strings.
- `packages/varlock/src/proxy/types.ts:471-477` classifies any matching `$NAME` string as a reference.
- `packages/varlock/src/env-graph/lib/env-graph.ts:1348-1357` strips `$`, changes `secretKey`, and omits the intended literal password.
- `packages/varlock/src/proxy/request-transform.ts:165-170` also treats every `$`-prefixed password as non-literal.

## Required outcome
- Quoted literal passwords must remain distinguishable from item references through validation, rule building, and signing.
- Add coverage for `secretIn="username"` with a literal password beginning with `$`, including a same-named config item to catch silent credential selection.

…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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ 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 $ITEM reference 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.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

Comment thread packages/varlock/src/proxy/docs-examples.test.ts Outdated
…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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No new issues in this incremental change; existing review threads remain open.

Reviewed changes since the prior Pullfrog review covered the HTTP Basic credential-model rewrite and its updated guidance and tests.

  • Simplified HTTP Basic credentials: Replaced secretIn with symmetric username and password slots, either of which can hold a literal or consumed $ITEM reference.
  • Changed attached-rule placement: Made the decorated item fill the first unset Basic-auth side, defaulting a lone credential to the userid position.
  • Expanded multi-credential coverage: Added graph and signer cases for token-as-userid, fixed usernames or passwords, empty userids, and two referenced credentials.
  • Updated user-facing descriptions: Revised CLI output, docs, and the release entry for the new credential semantics.

Pullfrog  | Fix it ➔View workflow run | Using azure/gpt-5.6-sol𝕏

The `proxy rules` credential rendering reads this helper and has no test of
its own; the symmetric-http-basic refactor changed both. Pins consumed vs wire
classification for two-position (http-basic), attached-default, and
single-secret (hmac + wire keyId) rules.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ 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 getTransformRoleKeys for both HTTP Basic credential slots, the attached-item default, and HMAC consumed versus wire roles used by proxy rules output.

Pullfrog  | Fix it ➔View workflow run | Using 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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 $NAME strings 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.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using azure/gpt-5.6-sol𝕏

for (const [keyA, itemA] of itemByOption) {
for (const [keyB, itemB] of itemByOption) {
if (keyA < keyB && itemA === itemB) {
return `transform.${keyA} and transform.${keyB} both reference "${itemA}"; each credential role needs its own item (one of these is sent upstream, the other is consumed by the signer)`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rejects any two options that reference the same item, even when both roles are consumed or both are wire; for example, http-basic declares both slots as consumed, so using one credential for both is safe but now fails with an inaccurate exposure error. Restrict the collision check to pairs whose roles differ and include a consumed-to-wire transition.

Technical details
# Preserve valid same-role credential reuse

## Affected sites
- `packages/varlock/src/proxy/types.ts:541-549` collects item names without retaining each option's `itemRole`, then rejects every duplicate.
- `packages/varlock/src/proxy/types.ts:370-377` defines both HTTP Basic slots as consumed credentials.
- `packages/varlock/src/env-graph/test/proxy-mode.test.ts:637-649` covers only the unsafe HMAC consumed/wire collision.

## Required outcome
- Reject reuse only when the same item crosses a consumed and wire-visible role.
- Continue allowing one item in multiple same-role positions for built-in and plugin schemes.
- Add a same-role regression case so the security guard does not expand into a general uniqueness constraint.

…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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ 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 $ITEM references.
  • Added graph coverage: Verified allowed non-sensitive interpolation and rejection of a sensitive value nested in an HTTP Basic credential option.

Pullfrog  | Fix it ➔View workflow run | Using azure/gpt-5.6-sol𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant