[highb]

Squire (openai/gpt-6.1-sol):

Declare canonical api_key_v2 before dispatch on the real Snowflake programmatic-access-token action. The action retains its matching JSON payload and provider handle. Native TOKEN still returns raw PAT bytes; the accepted name-based revoke limits remain explicit.

This unit reuses the existing action implementation #160 and integrates current main's newer SQL/client/dependency fixes. It consumes SDK #1192 at an immutable unreleased pseudo-version. Canonical definitions are exported in Multipass #975 and consumed by C1 #28031. These are source dependencies, not release or activation claims.

TestSnowflakeDeclaredSecretProof runs actual connector registration/schema lookup, action handler, SQL HTTP client, canonical encoder and SDK JWE. A controlled random-loopback provider handles DESCRIBE, GRANTS, ADD and SHOW. Old opaque/generic context and wrong purpose/version refuse before any provider SQL; success creates once and returns encrypted canonical bytes. The demo performs no revoke/use. A test-only file port accepts actual C1-prepared requests and exports the actual schema/JWE for the cross-repository proof.

Validation: full go test -p 2 -v ./... -count=1 passes, including native API_KEY/raw TOKEN, SQL pagination and name-based revoke controls. The live TestUserBuilderList is intentionally skipped because no provider credentials or calls are authorized. Source and vendored SDK output are separate commits.

Remaining draft acceptance: actual handler bytes through the replacement C1 frozen-type/member/Store/reveal stack and real Multipass codec/renderer. Existing synthetic-handler proof is baseline only. This change adds no new provider lifecycle or live operation.

[github-actions]

Superseded — see the current review report for commit feef135c0981

Connector PR Review: Declare canonical Snowflake secret action output type

Blocking Issues: 0 | Suggestions: 3 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base ac2ec67e0f94.
Review mode: full
View review run

Review Summary

What it adds:

• An issue_programmatic_access_token user resource action. It declares a secret return with content type api_key_v2, and the vendored SDK refuses the call before any provider SQL unless the JWE AAD carries a matching c1/fk-credential-delivery v1 context.
• An API_KEY issuance option that wraps the same PAT as {key_value, provider, header_name} JSON. TOKEN still returns raw bytes.

What it changes:

• PAT revoke is now strict: IF EXISTS is gone, and Delete checks that the parent matches the recorded user.
• Orphan cleanup in Issue now only runs for tokens that readback located.

Dependencies: baton-sdk moves to an unreleased pseudo-version.

Coverage:

• I scanned the full PR diff, including the vendored SDK files (actions.go, secret_content_type.go, regenerated config.pb*) and go.mod/go.sum, for security and correctness. I did not do a line-by-line review of the regenerated protobuf code.
• Repo-local criteria applied:
  • S/R7: no resource-producing method was changed to return empty results.
  • L1/L6: the old Error log on cleanup failure is replaced by a returned error.
  • P3: revoke idempotency, see Suggestions.
  • E/PR: entity source and arg order are correct.
  • D1–D4: docs/connector.mdx is updated for the new option and action.
  • Section C (spans) and section D (JSON types) don't apply.

Security Issues

None found. SQL identifiers go through quoteIdentifier, and request_id is limited to ^[A-Za-z0-9_-]{1,128}$. The PAT never appears in the public response, and the token is not logged.

Correctness Issues

None found.

Suggestions

• New (medium): pkg/connector/users.go:179-180. When readback succeeds but doesn't return the new token, or SHOW fails with a non-privilege error, the created PAT is now left live and its plaintext is discarded. The old deferred IF EXISTS removal used to clean it up here.
• New (medium): pkg/connector/programmatic_access_tokens.go:89. Strict REMOVE makes revoke non-idempotent (P3). A retry after a lost-but-successful REMOVE, or a revoke of an already-deleted PAT, fails permanently instead of converging.
• New (high): go.mod:8. baton-sdk is pinned to an unreleased pseudo-version from unmerged SDK PR #1192. Re-pin to a tagged release before merging. Promoting filippo.io/age/hpke to direct deps is test-only and justified.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/connector/users.go`:
- Around line 152-197: After CreateProgrammaticAccessToken succeeds, cleanup now only runs when `located` is true. When the readback list omits the token (the !found branch) or SHOW fails with a non-privilege error (default branch), the live PAT is left behind and its plaintext is discarded. Restore a best-effort removal for these paths, using RemoveProgrammaticAccessToken (IF EXISTS) since the name was just minted from this request. Otherwise, log a Warn with username and token_name so operators can find the orphan.

In `pkg/connector/programmatic_access_tokens.go`:
- Around line 85-94: Delete uses RemoveProgrammaticAccessTokenStrict, so a revoke retry after a successful-but-lost REMOVE, or a revoke of an already-deleted PAT, fails permanently. Detect Snowflake's "does not exist" error and either treat it as success or return a codes.NotFound status, keeping the explanatory message.

In `go.mod`:
- Around line 8: baton-sdk is pinned to pseudo-version v0.42.4-0.20261023000147-899cb2f0457c from an unmerged SDK PR. Once SDK #1192 is released, bump to the tagged version, then re-vendor and re-run tests.

[github-actions]

🟡 Suggestion (confidence: medium): Before this change, the deferred REMOVE ... IF EXISTS cleaned up a token when readback didn't return it. Now located stays false on this path, so the PAT is left live, the plaintext is thrown away, and nothing records a handle that could revoke it. The default: branch (any non-privilege SHOW error) has the same problem. If the name-reuse risk is the reason, consider keeping a best-effort IF EXISTS removal here, since the name was minted from this request a moment ago. Otherwise, emit a structured signal (e.g. a Warn with user and token name) so the orphan can be found.

[github-actions]

🟡 Suggestion (confidence: medium): Removing IF EXISTS means revoke is no longer idempotent (connector rule P3). Suppose a first REMOVE succeeds but the response is lost, or the PAT was already deleted in Snowflake. Every C1 retry of revoke then fails, and the record can't converge until the next sync drops it. The trade-off is documented, but consider mapping Snowflake's "does not exist" error to success, or to codes.NotFound, so callers can tell "already gone" apart from a real failure.

[github-actions]

🟡 Suggestion (confidence: high): baton-sdk is pinned to an unreleased pseudo-version from an unmerged SDK PR (#1192). The typed-output API (SecretContentType, PlaintextData.SetSchema, AAD validation) could still change before release. Please re-pin to a tagged SDK release before merging. The filippo.io/age/hpke promotion to direct deps is test-only and justified.

[github-actions]

Connector PR Review: Declare canonical Snowflake secret action output type

Blocking Issues: 0 | Suggestions: 3 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base ac2ec67e0f94.
Review mode: full
View review run

Review Summary

What it adds:

• An issue_programmatic_access_token user resource action. It declares a secret return with content type api_key_v2, and the vendored SDK refuses the call before any provider SQL unless the JWE AAD carries a matching c1/fk-credential-delivery v1 context.
• An API_KEY issuance option that wraps the same PAT as {key_value, provider, header_name} JSON. TOKEN still returns raw bytes.

What it changes:

• PAT revoke is now strict: IF EXISTS is gone, and Delete checks that the parent matches the recorded user.
• Orphan cleanup in Issue now only runs for tokens that readback located.

Dependencies: baton-sdk moves to an unreleased pseudo-version.

Coverage:

• I scanned the full PR diff, including the vendored SDK files (actions.go, secret_content_type.go, regenerated config.pb*) and go.mod/go.sum, for security and correctness. I did not do a line-by-line review of the regenerated protobuf code.
• Repo-local criteria applied:
  • S/R7: no resource-producing method was changed to return empty results.
  • L1/L6: the old Error log on cleanup failure is replaced by a returned error.
  • P3: revoke idempotency, see Suggestions.
  • E/PR: entity source and arg order are correct.
  • D1–D4: docs/connector.mdx is updated for the new option and action.
  • Section C (spans) and section D (JSON types) don't apply.

Security Issues

None found. SQL identifiers go through quoteIdentifier, and request_id is limited to ^[A-Za-z0-9_-]{1,128}$. The PAT never appears in the public response, and the token is not logged.

Correctness Issues

None found.

Suggestions

• New (medium): pkg/connector/users.go:179-180. When readback succeeds but doesn't return the new token, or SHOW fails with a non-privilege error, the created PAT is now left live and its plaintext is discarded. The old deferred IF EXISTS removal used to clean it up here.
• New (medium): pkg/connector/programmatic_access_tokens.go:89. Strict REMOVE makes revoke non-idempotent (P3). A retry after a lost-but-successful REMOVE, or a revoke of an already-deleted PAT, fails permanently instead of converging.
• New (high): go.mod:8. baton-sdk is pinned to an unreleased pseudo-version from unmerged SDK PR #1192. Re-pin to a tagged release before merging. Promoting filippo.io/age/hpke to direct deps is test-only and justified.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/connector/users.go`:
- Around line 152-197: After CreateProgrammaticAccessToken succeeds, cleanup now only runs when `located` is true. When the readback list omits the token (the !found branch) or SHOW fails with a non-privilege error (default branch), the live PAT is left behind and its plaintext is discarded. Restore a best-effort removal for these paths, using RemoveProgrammaticAccessToken (IF EXISTS) since the name was just minted from this request. Otherwise, log a Warn with username and token_name so operators can find the orphan.

In `pkg/connector/programmatic_access_tokens.go`:
- Around line 85-94: Delete uses RemoveProgrammaticAccessTokenStrict, so a revoke retry after a successful-but-lost REMOVE, or a revoke of an already-deleted PAT, fails permanently. Detect Snowflake's "does not exist" error and either treat it as success or return a codes.NotFound status, keeping the explanatory message.

In `go.mod`:
- Around line 8: baton-sdk is pinned to pseudo-version v0.42.4-0.20261023000147-899cb2f0457c from an unmerged SDK PR. Once SDK #1192 is released, bump to the tagged version, then re-vendor and re-run tests.

---

Reviewed commit: feef135c0981

[github-actions]

No blocking issues found — see the full review report