Skip to content

Commit 56c4983

Browse files
rustyconoverclaude
andcommitted
docs(attach): drop vgi_attach secrets; credentials pass inline as secret options
The DuckDB extension will not ship a vgi_attach secret type. Secret attach options are passed inline; the extension redacts them from duckdb_databases(), keeps only a salted hash in its cache key and never logs them. To keep a credential out of the SQL text, write the option as an expression, e.g. api_key getenv('SALES_API_KEY'). Credential options must still be declared secret=True. No behaviour change. Also exempt the C++ location_policy and attach_secrets integration areas in the conformance directory-parity check: both are enforced inside the extension, and the Python side of secret options is covered by tests/catalog/test_attach_option.py. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent df46e65 commit 56c4983

6 files changed

Lines changed: 32 additions & 20 deletions

File tree

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ Set `SENTRY_DSN` (and install `vgi[sentry]`) to forward unhandled exceptions to
392392

393393
**Attach options redaction:** by default *no* options are logged in the `catalog.attach` / `catalog.create` breadcrumbs because options routinely carry credentials. Implementers opt in via `CatalogInterface.loggable_attach_options(options) -> Mapping`, which returns a redacted, safe-to-log subset (host, region, bucket — never password/token/secret). When the override returns an empty mapping (the default), the `options` field is omitted from the breadcrumb entirely. See `docs/catalog-interface.md` for details.
394394

395-
**Secret attach options:** credential options (API keys, tokens, passwords) MUST be declared `AttachOption(..., secret=True)`. `AttachOptionSpec.secret` rides as a nullable `bool` column named `secret` appended after `required` (an absent column reads as `False`); clients and the DuckDB extension mask such options, keep them out of cache keys and logs, and can supply them from a `vgi_attach` DuckDB secret (`CREATE SECRET (TYPE vgi_attach, SCOPE '<worker url>', api_key '…')`). Secret + required is fine; secret + default is allowed but discouraged.
395+
**Secret attach options:** credential options (API keys, tokens, passwords) MUST be declared `AttachOption(..., secret=True)`. `AttachOptionSpec.secret` rides as a nullable `bool` column named `secret` appended after `required` (an absent column reads as `False`); they are passed inline at ATTACH; clients mask them, and the DuckDB extension redacts them from `duckdb_databases()`, keeps only a salted hash of them in its cache key, and never logs them. Keep a credential out of the SQL text with an expression: `ATTACH 'sales' (TYPE vgi, LOCATION '…', api_key getenv('SALES_API_KEY'))`. Secret + required is fine; secret + default is allowed but discouraged.
396396

397397
**Releases.** `vgi-serve` populates `release` from `SENTRY_RELEASE` if set, otherwise from `importlib.metadata.version("vgi-python")`. Production deploys should set `SENTRY_RELEASE=$(git rev-parse HEAD)` (or a tagged release identifier) so Sentry's commit-tracking and regression-detection features can correlate events to specific commits. Publishing GitHub releases makes the release-comparison UI more useful.
398398

‎docs/catalog-interface.md‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,11 +267,10 @@ class MyWorker(Worker):
267267
| `required=True` | The caller must supply the option. Declare it with no class-level default; `required` plus a default is rejected. |
268268
| `secret=True` | The option carries a credential. |
269269

270-
**Credential options (API keys, tokens, passwords) MUST be declared `secret=True`.** Clients and the DuckDB extension then mask the value, keep it out of result-cache keys and logs, and can supply it from a `vgi_attach` DuckDB secret, so the `ATTACH` statement never carries it:
270+
**Credential options (API keys, tokens, passwords) MUST be declared `secret=True`.** The caller passes them inline as attach options. Clients mask secret options, and the DuckDB extension redacts them from `duckdb_databases()`, keeps only a salted hash of them in its result-cache key, and never logs them. To keep a credential out of the SQL text (shell history, scripts, shared links), write the option as an expression:
271271

272272
```sql
273-
CREATE SECRET (TYPE vgi_attach, SCOPE 'https://worker.example.com', api_key '...');
274-
ATTACH 'mydb' AS mydb (TYPE vgi, LOCATION 'https://worker.example.com');
273+
ATTACH 'mydb' (TYPE vgi, LOCATION 'https://worker.example.com', api_key getenv('MYDB_API_KEY'));
275274
```
276275

277276
`secret` combines with `required`. It is allowed together with a default, but a secret option should normally have none: the default is advertised in plain text to every client at discovery.

‎tests/conformance/test_directory_parity.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,19 @@
7979
"(it spawns the Python worker as a subprocess); the Python client never "
8080
"exercises the launcher surface."
8181
),
82+
"attach_secrets": (
83+
"Masking, redaction from duckdb_databases() and cache-key hashing of secret "
84+
"attach options happen inside the C++ extension. The Python side is the "
85+
"AttachOptionSpec.secret wire column, covered by "
86+
"tests/catalog/test_attach_option.py (round trip, absent column, interaction "
87+
"with required)."
88+
),
89+
"location_policy": (
90+
"LOCATION transport policy (vgi_allowed_transports, enable_external_access) "
91+
"is enforced by the C++ extension before it starts or connects to a worker; "
92+
"it is a DuckDB setting with no counterpart in the Python client, which is "
93+
"handed a transport directly."
94+
),
8295
"simple_writable": (
8396
"INSERT/UPDATE/DELETE/RETURNING write paths are mirrored by "
8497
"test_writable.py, which drives the same operations against the writable "

‎vgi/catalog/attach_option.py‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@
1111
the machinery in ``vgi.catalog._descriptor_spec``.
1212
1313
Credential options (API keys, tokens, passwords) MUST be declared
14-
``secret=True``. Clients and the DuckDB extension then mask the value, keep it
15-
out of result-cache keys and logs, and can supply it from a ``vgi_attach``
16-
DuckDB secret instead of the ``ATTACH`` text::
14+
``secret=True``. The caller passes them inline as attach options; the DuckDB
15+
extension redacts them from ``duckdb_databases()``, keeps only a salted hash of
16+
them in its result-cache key, and never logs them. To keep a credential out of
17+
the SQL text, write the option as an expression::
1718
18-
CREATE SECRET (TYPE vgi_attach, SCOPE 'https://worker.example.com', api_key '...');
19-
ATTACH 'mydb' AS mydb (TYPE vgi, LOCATION 'https://worker.example.com');
19+
ATTACH 'mydb' (TYPE vgi, LOCATION 'https://worker.example.com', api_key getenv('MYDB_API_KEY'));
2020
"""
2121

2222
from dataclasses import dataclass
@@ -51,11 +51,12 @@ class AttachOptionSpec(_SpecBase):
5151
Mutually exclusive with ``default`` — an option that falls back to
5252
a value is by definition satisfiable without the caller.
5353
secret: The option carries a credential (an API key, token or
54-
password). Clients and the DuckDB extension mask its value, keep it
55-
out of result-cache keys and logs, and can supply it from a
56-
``vgi_attach`` DuckDB secret
57-
(``CREATE SECRET (TYPE vgi_attach, SCOPE '<worker url>', api_key '...')``)
58-
rather than the ``ATTACH`` text. Credential options MUST set this.
54+
password). It is still passed inline at ``ATTACH``, but clients
55+
mask it, and the DuckDB extension redacts it from
56+
``duckdb_databases()``, keeps only a salted hash of it in its
57+
result-cache key, and never logs it. Keep it out of the SQL text
58+
with an expression, e.g. ``api_key getenv('MYDB_API_KEY')``.
59+
Credential options MUST set this.
5960
Compatible with ``required``. Allowed alongside a ``default``, but a
6061
secret option should normally have none: a default credential is
6162
advertised in plain text to every client at discovery.
@@ -123,9 +124,8 @@ class AttachOptions:
123124
Attributes:
124125
required: The caller must supply this option at ``ATTACH`` time.
125126
secret: The option carries a credential. Clients and the DuckDB
126-
extension mask it, keep it out of cache keys and logs, and can
127-
supply it from a ``vgi_attach`` DuckDB secret. Should normally have
128-
no default.
127+
extension mask it, keep only a salted hash of it in cache keys, and
128+
never log it. Should normally have no default.
129129
130130
"""
131131

‎vgi/http/landing.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ <h1 id="worker-name">—</h1>
793793
'<table class="opts"><thead><tr><th>Option</th><th>Type</th><th>Default</th><th>Description</th></tr></thead><tbody>'+
794794
opts.map(o=>'<tr><td class="o-name">'+esc(o.name)+
795795
(o.required?'<span class="o-req" title="Must be supplied at ATTACH time">required</span>':'')+
796-
(o.secret?'<span class="o-req" title="A credential: supply it from a vgi_attach DuckDB secret rather than the ATTACH text">secret</span>':'')+
796+
(o.secret?'<span class="o-req" title="A credential: redacted and never logged. Keep it out of the SQL text with an expression such as getenv()">secret</span>':'')+
797797
'</td><td class="o-type">'+esc(o.type)+'</td><td class="o-def">'+esc(o.secret&&o.default?'hidden':(o.default||'—'))+'</td><td class="o-desc">'+esc(o.description||'')+'</td></tr>').join('')+
798798
'</tbody></table></details>';
799799
}

‎vgi/http/worker_page.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -597,8 +597,8 @@ def _build_attach_options_table(specs: tuple[AttachOptionSpec, ...]) -> str:
597597
flags += ' <span class="o-flag" title="Must be supplied at ATTACH time">required</span>'
598598
if spec.secret:
599599
flags += (
600-
' <span class="o-flag" title="A credential: supply it from a vgi_attach DuckDB secret '
601-
'rather than the ATTACH text">secret</span>'
600+
' <span class="o-flag" title="A credential: redacted and never logged. Keep it out of '
601+
'the SQL text with an expression such as getenv()">secret</span>'
602602
)
603603
parts.append(
604604
f"<tr><td><code>{_esc(spec.name)}</code>{flags}</td>"

0 commit comments

Comments
 (0)