Skip to content

Commit 0b993d2

Browse files
rustyconoverclaude
andcommitted
fix: return bind-time secrets from the batch the client actually sends
The secrets batch has one struct column per resolved secret, named by the secret's name, with type/provider/name/scope as fields. SecretsAccessor instead looked for dynamically requested secrets in `secret_N` columns with Arrow field metadata, a layout no client sends. On the second bind `params.secrets.get(type, name=...)` and `get(type, scope=...)` returned None although the secret was in the batch, and `required=True` raised. A table function's `Secret()` annotation looked for a column named by the secret type, so it got None unless the secret was named after its type. ResolvedSecrets.lookup(type, name=, scope=) now selects with the rule the client resolves by, and the accessor, the table-function annotation path and the scalar annotation path all use it. A secret named `secret_0` is an ordinary secret. docs/protocol/vgi-secrets.md writes the lookup protocol and the batch layout down; until now the layout was defined only by the DuckDB extension's BuildSecretsBatch. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 53517c1 commit 0b993d2

6 files changed

Lines changed: 418 additions & 209 deletions

File tree

‎docs/protocol/index.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ Each document declares its own status; “proposed” means the contract is stil
88
| [VGI Filter Encoding v2](vgi-filter-encoding-v2-spec.md) | **Normative, proposed** | Engine-neutral wire format, semantics, validation, and conformance requirements for `vgi.filters.v2`. |
99
| [VGI Runtime-Filter Artifacts](vgi-runtime-filter-artifacts.md) | **Normative optional extension, proposed** | Additional requirements for capability-gated advisory runtime-pruning artifacts. Base filter-v2 conformance does not require this extension. |
1010
| [VGI DuckDB Filter Adapter](vgi-duckdb-filter-adapter.md) | **Informative implementation guide, proposed** | DuckDB 1.5 and 2.0 mappings into the normative filter-v2 wire contract. |
11+
| [VGI Secrets](vgi-secrets.md) | **Normative** | How a worker asks for client-side secrets, the two-phase bind, and the layout of the secrets batch. |
1112
| [VGI catalog query pushdown](vgi-catalog-query-pushdown-design.md) | **Informative implementation design, proposed** | Updated 2026-10-06: proposed VGI 2.2 query preparation, DuckDB 1.5 explicit execution, and DuckDB 2.0 bound-plan SQL export for automatic pushdown. |
1213
| [Proposed VGI protocol changes for DuckDB 2.0](vgi-protocol-proposed-changes.md) | **Informative design/audit report** | Rationale, migration inventory, implementation status, and deferred protocol decisions. It is not an implementation specification. |
1314

‎docs/protocol/vgi-secrets.md‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
# VGI Secrets
2+
3+
**Status:** Normative. It records the contract the DuckDB extension and the SDKs already implement.
4+
**Protocol:** `vgi.v2`. No wire change.
5+
**Reference implementations:** client, `BuildSecretsBatch` and `ExtractVgiSecrets` in the DuckDB
6+
extension; worker, `SecretsAccessor` and `ResolvedSecrets` in vgi-python `vgi/table_function.py`.
7+
8+
The words MUST, MUST NOT and SHOULD are used as in RFC 2119.
9+
10+
## 1. Purpose
11+
12+
A function reads credentials that the user keeps in the client's secret store (in DuckDB,
13+
`CREATE SECRET`). The worker never reads that store. It says which secrets it needs, the client
14+
resolves them, and the resolved values travel to the worker inside the bind request.
15+
16+
Two things are specified here: how a worker asks for a secret, and the layout of the batch that
17+
carries the answers.
18+
19+
## 2. Asking for a secret
20+
21+
A lookup is a triple:
22+
23+
| Field | Meaning |
24+
|---|---|
25+
| `secret_type` | Required. The secret type, such as `s3`. |
26+
| `secret_name` | Optional. The name of one specific secret. |
27+
| `scope` | Optional. A path the secret must apply to, such as `s3://bucket/key`. |
28+
29+
A worker asks in one of two ways.
30+
31+
**Statically.** Function metadata lists lookups in `required_secrets`. The client resolves them
32+
before the first bind, so they are in the first bind request.
33+
34+
**Dynamically.** A bind may need a lookup it can only compute from its arguments, such as the bucket
35+
in a path argument. The worker answers the bind with a *secret-scope request*: a `BindResponse`
36+
whose `lookup_secret_types` list is non-empty. `lookup_scopes` and `lookup_names` are parallel
37+
lists of the same length, with the empty string where a lookup has no scope or no name. The other
38+
fields of that response are ignored.
39+
40+
The client resolves the requested lookups, adds them to the secrets it already resolved, and sends
41+
the bind again with `resolved_secrets_provided = true`. A worker MUST NOT answer that second bind
42+
with another secret-scope request; the client rejects it. A lookup that is still unanswered on the
43+
second bind does not exist.
44+
45+
## 3. How the client resolves a lookup
46+
47+
- **With a name:** the secret of that name, if its type equals `secret_type`. The scope is ignored.
48+
- **Without a name:** the secret of that type whose scope best matches `scope` under the client's
49+
own rule. DuckDB uses the longest scope that is a prefix of the path. An empty `scope` matches
50+
only secrets that have no scope.
51+
52+
A lookup that matches nothing is not an error for the client. The secret is left out of the batch
53+
and the worker decides what that means.
54+
55+
A secret type that sets a default scope is not matched by a lookup with an empty scope. DuckDB's
56+
`s3` secrets default to the scope `s3://`, so a worker looking for "the user's S3 secret" asks for
57+
scope `s3://`, not for the type alone.
58+
59+
## 4. The secrets batch
60+
61+
`BindRequest.secrets` is a binary field. It holds an Arrow IPC stream with one record batch, or is
62+
null when no secret was resolved.
63+
64+
The batch has **one row** and **one column per resolved secret**:
65+
66+
- The column is **named by the secret's name**, the name the user gave it or the client's default
67+
name. It is not named by the secret's type. Several secrets of one type therefore coexist, one
68+
per bucket for example.
69+
- The column is a **struct**. Its fields are the secret's own key/value pairs, each with the Arrow
70+
type of its value, plus the four fields below.
71+
- A secret that was resolved by more than one lookup appears once.
72+
73+
| Field | Type | Content |
74+
|---|---|---|
75+
| `type` | utf8 | The secret type. |
76+
| `provider` | utf8 | The provider that created the secret, such as `config` or `credential_chain`. |
77+
| `name` | utf8 | The secret name. Equal to the column name. |
78+
| `scope` | utf8 | The secret's scope prefixes joined by a newline (`\n`). Empty when it has none. |
79+
80+
Example, for two `s3` secrets named `prod` and `dev`:
81+
82+
```text
83+
prod: struct<key_id: utf8, name: utf8, provider: utf8, region: utf8, scope: utf8, secret: utf8, type: utf8>
84+
dev: struct<key_id: utf8, name: utf8, provider: utf8, region: utf8, scope: utf8, secret: utf8, type: utf8>
85+
```
86+
87+
Rules:
88+
89+
1. A client MUST name each column by the secret name and MUST include `type`, `name` and `scope`.
90+
2. A worker MUST find a secret by column name or by the `type` and `scope` fields. It MUST NOT
91+
depend on column order, on column position, or on Arrow field metadata.
92+
3. A worker MUST accept struct fields it does not know, and MUST treat a null column as an
93+
unresolved secret.
94+
4. The batch does not say which lookup produced which secret. A worker matches them itself (§5).
95+
5. The batch contains credentials in plain text. Neither side may log it or copy it into
96+
`opaque_data`.
97+
98+
The same batch is sent wherever a bind request is sent or embedded, which includes the bind request
99+
carried inside each init request. A function therefore sees the same secrets at execution time as
100+
at bind time.
101+
102+
## 5. How the worker finds its secret
103+
104+
A worker selects from the batch with the rule the client used, so that it picks the secret its own
105+
lookup resolved:
106+
107+
- **By name:** the column of that name, if its `type` field equals the requested type.
108+
- **By scope:** among secrets of the requested type, the one with the longest scope prefix of the
109+
requested path; failing that, one with an empty scope.
110+
- **By type alone:** a secret of that type with an empty scope; failing that, the first secret of
111+
that type.
112+
113+
SDKs expose these as selectors on the resolved-secrets object.
114+
115+
| SDK | By type | By scope and type |
116+
|---|---|---|
117+
| Python | `ResolvedSecrets.of_type`, `.lookup` | `.for_scope_of_type`, `.lookup` |
118+
| Rust | `Secrets::of_type` | `Secrets::for_scope_of_type` |
119+
| Go | `Secrets.OfType` | `Secrets.ForScopeOfType` |
120+
| C++ | `Secrets::of_type` | `Secrets::for_scope_of_type` |
121+
| Java | `Secrets.ofType` | `Secrets.forScopeOfType` |
122+
| C# | `SecretArgCodec.AllOfType` | `SecretArgCodec.ForScopeOfType` |
123+
| TypeScript | `secretsOfType` | `secretForScopeOfType` |
124+
125+
## 6. History
126+
127+
vgi-python's bind-time `SecretsAccessor` used to expect dynamically requested secrets in columns
128+
named `secret_0`, `secret_1`, … with the lookup's type, name and scope in Arrow field metadata. No
129+
client sends that layout, so `params.secrets.get(type, name=…)` and `get(type, scope=…)` returned
130+
`None` on the second bind although the secret was in the batch. The accessor now reads the layout
131+
in §4, and `secret_0` is an ordinary secret name.

‎mkdocs.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ nav:
150150
- "Catalog query pushdown (proposed design)": protocol/vgi-catalog-query-pushdown-design.md
151151
- "DuckDB 2.0 design audit (informative)": protocol/vgi-protocol-proposed-changes.md
152152
- "Attach tickets (normative)": protocol/vgi-attach-tickets.md
153+
- "Secrets (normative)": protocol/vgi-secrets.md
153154
- API Reference:
154155
- api/index.md
155156
- Functions: api/functions.md

‎tests/test_setting_secret_annotations.py‎

Lines changed: 148 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from typing import Annotated, Any
1010

1111
import pyarrow as pa
12+
import pytest
1213

1314
from tests.conftest import FIXTURE_WORKER
1415
from vgi.arguments import (
@@ -418,6 +419,16 @@ def test_bind_params_secrets_type(self) -> None:
418419
assert len(outputs) > 0
419420

420421

422+
def _wire_secrets(*secrets: dict[str, str]) -> pa.RecordBatch:
423+
"""Build a secrets batch the way the DuckDB extension does.
424+
425+
One row, one struct column per secret, the column named by the secret's
426+
``name``; ``type``, ``provider``, ``name`` and ``scope`` are fields.
427+
"""
428+
columns = {s["name"]: [{"provider": "config", **s}] for s in secrets}
429+
return pa.RecordBatch.from_pydict(columns)
430+
431+
421432
# ---------------------------------------------------------------------------
422433
# Tests for SecretsAccessor
423434
# ---------------------------------------------------------------------------
@@ -461,57 +472,157 @@ def test_scoped_get_registers_pending(self) -> None:
461472
assert entry.secret_type == "s3"
462473
assert entry.scope == "s3://bucket/"
463474

464-
def test_to_dict_combines_unscoped_and_scoped(self) -> None:
465-
"""to_dict() includes both unscoped and scoped entries."""
466-
# Build a batch with an unscoped column and a scoped column
467-
struct_type = pa.struct([("k", pa.string())])
468-
unscoped_arr = pa.array([{"k": "unscoped_val"}], type=struct_type)
469-
470-
scoped_field = pa.field(
471-
"secret_0",
472-
struct_type,
473-
metadata={"secret_type": "scoped_type", "scope": "s3://x/"},
475+
def test_to_dict_is_keyed_by_secret_name(self) -> None:
476+
"""to_dict() keeps every resolved secret under its own name, nulls dropped."""
477+
batch = _wire_secrets(
478+
{"type": "s3", "name": "prod", "scope": "s3://prod/", "key_id": "P"},
479+
{"type": "s3", "name": "dev", "scope": "s3://dev/", "key_id": "D"},
474480
)
475-
scoped_arr = pa.array([{"k": "scoped_val"}], type=struct_type)
481+
struct_type = pa.struct([("type", pa.string())])
482+
batch = batch.append_column(pa.field("gone", struct_type), pa.array([None], type=struct_type))
483+
result = SecretsAccessor(batch).to_dict()
484+
assert sorted(result) == ["dev", "prod"]
485+
assert result["prod"]["key_id"].as_py() == "P"
476486

477-
batch = pa.RecordBatch.from_arrays(
478-
[unscoped_arr, scoped_arr],
479-
schema=pa.schema([pa.field("my_type", struct_type), scoped_field]),
487+
def test_a_secret_named_like_the_old_wire_columns_is_an_ordinary_secret(self) -> None:
488+
"""A user may name a secret ``secret_0``; it is not a special column."""
489+
batch = _wire_secrets({"type": "s3", "name": "secret_0", "scope": "s3://", "key_id": "K"})
490+
accessor = SecretsAccessor(batch, is_retry=True)
491+
assert accessor.to_dict()["secret_0"]["key_id"].as_py() == "K"
492+
found = accessor.get("s3", name="secret_0")
493+
assert found is not None
494+
assert found["key_id"].as_py() == "K"
495+
496+
def test_get_by_name_on_retry_returns_the_secret(self) -> None:
497+
"""The two-phase bind: a by-name lookup registers, then the retry returns it.
498+
499+
Regression: the retry looked for ``secret_N`` columns that no client
500+
sends, so the lookup came back ``None`` although the secret was in the
501+
batch, and ``required=True`` raised.
502+
"""
503+
first = SecretsAccessor(None)
504+
assert first.get("s3", name="prod", required=True) is None
505+
assert first.pending_lookups == [SecretLookupEntry(secret_type="s3", secret_name="prod")]
506+
507+
batch = _wire_secrets({"type": "s3", "name": "prod", "scope": "s3://", "key_id": "P"})
508+
retry = SecretsAccessor(batch, is_retry=True)
509+
found = retry.get("s3", name="prod", required=True)
510+
assert found is not None
511+
assert found["key_id"].as_py() == "P"
512+
assert retry.all_resolved
513+
514+
def test_get_by_name_checks_the_type(self) -> None:
515+
"""A secret with the right name but another type is not a match."""
516+
batch = _wire_secrets({"type": "gcs", "name": "prod", "scope": "gcs://"})
517+
accessor = SecretsAccessor(batch, is_retry=True)
518+
assert accessor.get("s3", name="prod") is None
519+
with pytest.raises(ValueError, match="Required secret 's3' not found"):
520+
accessor.get("s3", name="prod", required=True)
521+
522+
def test_get_by_name_already_resolved_needs_no_retry(self) -> None:
523+
"""A by-name lookup the client pre-resolved is answered on the first bind."""
524+
batch = _wire_secrets({"type": "s3", "name": "prod", "scope": "s3://", "key_id": "P"})
525+
accessor = SecretsAccessor(batch)
526+
assert accessor.get("s3", name="prod") is not None
527+
assert not accessor.needs_resolution
528+
529+
def test_get_by_scope_on_retry_picks_the_longest_prefix(self) -> None:
530+
"""A scope lookup returns the secret whose scope is the longest prefix."""
531+
batch = _wire_secrets(
532+
{"type": "s3", "name": "any", "scope": "s3://", "key_id": "ANY"},
533+
{"type": "s3", "name": "prod", "scope": "s3://prod/\ns3n://prod/", "key_id": "PROD"},
534+
{"type": "gcs", "name": "other", "scope": "s3://prod/data/", "key_id": "WRONG-TYPE"},
480535
)
481-
result = SecretsAccessor(batch).to_dict()
482-
assert "my_type" in result
483-
assert result["my_type"]["k"].as_py() == "unscoped_val"
484-
assert "scoped_type" in result
485-
assert result["scoped_type"]["k"].as_py() == "scoped_val"
486-
487-
def test_to_dict_skips_null_scoped(self) -> None:
488-
"""to_dict() skips scoped entries with null values."""
489-
struct_type = pa.struct([("k", pa.string())])
490-
scoped_field = pa.field(
491-
"secret_0",
492-
struct_type,
493-
metadata={"secret_type": "null_type"},
536+
accessor = SecretsAccessor(batch, is_retry=True)
537+
prod = accessor.get("s3", scope="s3://prod/data/x.parquet", required=True)
538+
assert prod is not None and prod["key_id"].as_py() == "PROD"
539+
other = accessor.get("s3", scope="s3://elsewhere/x.parquet", required=True)
540+
assert other is not None and other["key_id"].as_py() == "ANY"
541+
assert accessor.get("s3", scope="az://nope/") is None
542+
543+
def test_get_by_scope_always_asks_the_client_first(self) -> None:
544+
"""Only the client knows whether a longer-scoped secret exists."""
545+
batch = _wire_secrets({"type": "s3", "name": "any", "scope": "s3://", "key_id": "ANY"})
546+
accessor = SecretsAccessor(batch)
547+
assert accessor.get("s3", scope="s3://prod/x") is None
548+
assert accessor.pending_lookups == [SecretLookupEntry(secret_type="s3", scope="s3://prod/x")]
549+
550+
def test_get_by_type_finds_a_secret_whatever_its_name(self) -> None:
551+
"""A by-type lookup matches the ``type`` field, not the column name."""
552+
batch = _wire_secrets(
553+
{"type": "vgi_example", "name": "scoped", "scope": "x://a/", "v": "scoped"},
554+
{"type": "vgi_example", "name": "__default_vgi_example", "scope": "", "v": "default"},
555+
{"type": "s3", "name": "aaa", "scope": "", "v": "s3"},
494556
)
495-
scoped_arr = pa.array([None], type=struct_type)
496-
batch = pa.RecordBatch.from_arrays(
497-
[scoped_arr],
498-
schema=pa.schema([scoped_field]),
557+
accessor = SecretsAccessor(batch)
558+
found = accessor.get("vgi_example")
559+
assert found is not None
560+
assert found["v"].as_py() == "default"
561+
assert not accessor.needs_resolution
562+
563+
def test_get_by_type_falls_back_to_a_scoped_secret(self) -> None:
564+
"""With no unscoped secret of the type, the first one of that type is used."""
565+
batch = _wire_secrets({"type": "s3", "name": "prod", "scope": "s3://", "v": "p"})
566+
found = SecretsAccessor(batch).get("s3")
567+
assert found is not None
568+
assert found["v"].as_py() == "p"
569+
570+
def test_annotated_secret_is_selected_by_name_and_scope(self) -> None:
571+
"""A ``Secret()`` annotation on on_bind gets the secret it names, through a real bind.
572+
573+
Regression: the table-function path looked for a column named by the
574+
secret *type*, so an annotated secret arrived as ``None`` unless the
575+
user had named the secret after its type.
576+
"""
577+
from vgi.arguments import Arguments
578+
from vgi.invocation import FunctionType
579+
from vgi.protocol import BindRequest
580+
581+
seen: dict[str, Any] = {}
582+
583+
@dataclass(slots=True, frozen=True)
584+
class _Args:
585+
count: Annotated[int, Arg(0, doc="Count")] = 10
586+
587+
class _TestFunc(TableFunctionGenerator[_Args]):
588+
@classmethod
589+
def on_bind(
590+
cls,
591+
params: BindParams[_Args],
592+
*,
593+
by_name: Annotated[dict[str, pa.Scalar[Any]] | None, Secret("s3", name="dev")] = None,
594+
by_scope: Annotated[dict[str, pa.Scalar[Any]] | None, Secret("s3", scope="s3://prod/x")] = None,
595+
missing: Annotated[dict[str, pa.Scalar[Any]] | None, Secret("gcs")] = None,
596+
) -> BindResponse:
597+
seen.update(by_name=by_name, by_scope=by_scope, missing=missing)
598+
return BindResponse(output_schema=pa.schema([pa.field("x", pa.int64())]))
599+
600+
@classmethod
601+
def process(cls, params: Any, state: Any, out: Any) -> None: ...
602+
603+
request = BindRequest(
604+
function_name="_test_func",
605+
arguments=Arguments(positional=(pa.scalar(1),)),
606+
function_type=FunctionType.TABLE,
607+
secrets=_wire_secrets(
608+
{"type": "s3", "name": "dev", "scope": "s3://dev/", "key_id": "DEV"},
609+
{"type": "s3", "name": "prod", "scope": "s3://prod/", "key_id": "PROD"},
610+
),
499611
)
500-
result = SecretsAccessor(batch).to_dict()
501-
assert "null_type" not in result
612+
_TestFunc.bind(request)
613+
614+
assert seen["by_name"]["key_id"].as_py() == "DEV"
615+
assert seen["by_scope"]["key_id"].as_py() == "PROD"
616+
assert seen["missing"] is None
502617

503618
def test_required_raises_on_retry(self) -> None:
504619
"""required=True raises ValueError when secret is genuinely missing on retry."""
505-
import pytest
506-
507620
accessor = SecretsAccessor(None, is_retry=True)
508621
with pytest.raises(ValueError, match="Required secret"):
509622
accessor.get("missing_type", required=True)
510623

511624
def test_scoped_required_raises_on_retry(self) -> None:
512625
"""required=True with scope raises ValueError on retry when not found."""
513-
import pytest
514-
515626
accessor = SecretsAccessor(None, is_retry=True)
516627
with pytest.raises(ValueError, match="Required secret"):
517628
accessor.get("s3", scope="s3://bucket/", required=True)
@@ -557,7 +668,6 @@ def test_non_serializable_state_raises(self) -> None:
557668
still rejected, and rejected at class-definition time rather than
558669
silently falling back to initial_state() on every HTTP exchange.
559670
"""
560-
import pytest
561671

562672
@dataclass
563673
class BadState:

0 commit comments

Comments
 (0)