Skip to content

[feat](distributed) Add scoped secret plan references - #604

Draft
kaka11chen wants to merge 4 commits into
AstroVela:mainfrom
kaka11chen:feat/issue-547-distributed-secrets-manager
Draft

[feat](distributed) Add scoped secret plan references#604
kaka11chen wants to merge 4 commits into
AstroVela:mainfrom
kaka11chen:feat/issue-547-distributed-secrets-manager

Conversation

@kaka11chen

@kaka11chen kaka11chen commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a versioned, opaque ScopedSecretRef contract with deterministic UUIDv8 identity, query/session ownership, DuckDB-selected normalized scope, and read/write capabilities; secret names and credential values never enter the plan or pickle state.
  • Discover only referenced object-storage uses, retain both matched and initially-unmatched source uses for transport-time freshness checks, and propagate refs through logical/physical planning, strict pickle state, clones, deferred execution, query replay, and worker task reconstruction.
  • Prove generated remote COPY credential selection across both the canonical data namespace and the sibling .duckdb_commit lifecycle namespace; hard-reject nonstandard lookup semantics, nested selection changes, stale registry state, unsupported providers, ambiguous refs, malformed input, and resource-limit violations.
  • Add metadata-only DuckDB secret scanning that does not clone credential-bearing entries, standard lookup certificates, storage-registry generation checks, bounded/redacted validation, and custom DUCKDB_SOURCE_PATH-correct mbedTLS includes.

This is the inactive discovery/transport foundation from #588. It does not resolve, refresh, or install credentials on Ray workers and does not yet enable user-visible multi-scope distributed I/O; worker resolution remains a later broker task. No compatibility or ambient-credential fallback state is carried.

Related issue

close: #588

Documentation impact

  • No user-facing documentation update is required.
  • Documentation is updated in this PR or in a linked website PR.
  • A follow-up issue is required in
    AstroVela/vane-website.

The contract remains internal and inactive until the worker broker/resolution work is implemented.

Validation

  • Two consecutive independent clean review rounds on the unchanged final HEAD after all review findings were fixed; each round covered C++ API/call-site closure, ref propagation and serialization, COPY data/commit paths, selection proofs, concurrency, limits, and redaction.
  • scripts/format workspace --changed
  • Non-editable incremental Release install with SKBUILD_BUILD_DIR="$PWD/build/python-release", SKBUILD_CMAKE_BUILD_TYPE=Release, and uv pip install . --no-build-isolation.
  • DuckDB unittest "[secret]" — 14 test cases passed, 159 assertions; 5 S3-server-dependent cases skipped.
  • scripts/run_installed_pytest.sh tests/fast/test_scoped_secret_refs.py tests/fast/test_ray_connection_snapshot.py — 84 passed.
  • scripts/run_release_tests.sh — non-Ray: 553 passed, 15 deselected; real Ray: 11 passed, 557 deselected.

Checklist

  • The change is focused and includes tests or a reason tests are unnecessary.
  • Public behavior and compatibility impact are documented.
  • New dependencies, copied code, model assets, and datasets have compatible
    licenses and are recorded where required.
  • No credentials, private endpoints, personal paths, generated data, model
    weights, or build artifacts are included.
  • Security implications of UDFs, serialization, remote code, network access,
    and untrusted input have been considered.
  • Native changes were compiled; Python-only, documentation, and workflow
    changes passed relevant checks.

Copilot AI lite review requested due to automatic review settings August 16, 2026 08:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7e722907cc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/vane_py/ray/logical_plan_bindings.cpp
Comment thread src/vane_py/ray/logical_plan_bindings.cpp Outdated
Comment thread src/vane_py/ray/CMakeLists.txt Outdated
Comment thread src/vane_py/ray/logical_plan_bindings.cpp Outdated
@kaka11chen
kaka11chen force-pushed the feat/issue-547-distributed-secrets-manager branch from 7e72290 to 9d91c17 Compare August 16, 2026 10:49
@kaka11chen

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 9d91c17ba6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kaka11chen
kaka11chen marked this pull request as draft August 16, 2026 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Secrets 1/5] Add opaque scoped-secret references to distributed plans

2 participants