Skip to content

feat(core): add listSecrets, getSecret, updateSecret, deleteSecret SDK functions - #139

Merged
birme merged 1 commit into
mainfrom
feat/4371-secret-list-get-delete
Jun 3, 2026
Merged

birme merged 1 commit into
mainfrom
feat/4371-secret-list-get-delete

Conversation

@birme

@birme birme commented Jun 3, 2026

Copy link
Copy Markdown
Member

Summary

Adds the missing secret lifecycle operations to @osaas/client-core. The backend (deploy-manager) already supports these endpoints; this PR closes the SDK gap.

  • listSecrets(serviceId, ctx) — GET /mysecrets/:serviceId, returns metadata only (no values)
  • getSecret(serviceId, secretName, ctx) — client-side list + find (no single-secret endpoint)
  • updateSecret(serviceId, secretName, value, ctx) — PUT /mysecrets/:serviceId/:secretName (upsert)
  • deleteSecret(serviceId, secretName, ctx) — DELETE /mysecrets/:serviceId/:secretName

Also refactors packages/cli/src/user/cmd.ts cmdSecrets to use the new SDK functions instead of inline createFetch calls.

Closes Eyevinn/osaas-app#4371.

Lessons

Secret values are write-only by design — list and get return only metadata (serviceId, secretName). The getSecret function is implemented client-side via listSecrets + .find() because no single-secret GET endpoint exists on the backend. Document this in JSDoc to prevent future agents from trying to add a "get secret value" endpoint. The cmdSecrets refactor also revealed that saveSecret in core.ts uses name as the parameter but passes it as secretName in the body — matching the existing POST contract. The new updateSecret uses secretName as the URL path segment (matching the CLI's PUT pattern) rather than a body field.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

… client-core (closes osaas-app#4371)

Co-Authored-By: Claude <noreply@anthropic.com>
@birme birme self-assigned this Jun 3, 2026
@birme

birme commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

Research Review

Tier classification: Tier 3 (touches auth/token handling, shared SDK library with external consumers, API contract changes)


Verified

  • Endpoint paths: GET /mysecrets/:serviceId, PUT /mysecrets/:serviceId/:secretName, DELETE /mysecrets/:serviceId/:secretName all confirmed in src/api_mysecrets.ts of Eyevinn/osaas-deploy-manager.
  • Auth header pattern: x-pat-jwt: Bearer <token> confirmed against src/api_mysecrets.ts — the extractPat function matches exactly ^Bearer (\S+)$.
  • getPersonalAccessToken() is synchronous: Confirmed in packages/core/src/context.ts:42 — no await needed, consistent with every other function in core.ts. The new functions all call it correctly.
  • getSecret uses listSecrets + .find(): Confirmed correct by design — no single-secret GET endpoint exists in the backend router. JSDoc note matches reality.
  • CLI refactoring faithfulness: The removed inline createFetch blocks in cmd.ts used identical paths, methods, and body shapes to what the new SDK functions send. listSecrets replaces GET /mysecrets/:serviceId, updateSecret replaces PUT /mysecrets/:serviceId/:secretName, deleteSecret replaces DELETE /mysecrets/:serviceId/:secretName.
  • updateSecret body field: The CLI was sending { secretData: secretValue }. The backend SecretUpdate type is Type.Omit(Secret, ['serviceId', 'secretName']), which resolves to { secretData: string, lastUpdated?: string }. The new SDK sends { secretData: value } — correct.
  • saveSecret POST body: The existing function sends { secretName: name, secretData: value } matching SecretRequest = Type.Omit(Secret, ['serviceId']) — this was not changed and is correct.
  • Lessons section: Present and filled in with substantive non-obvious findings.

Needs Verification

  • listSecrets return type is under-typed (packages/core/src/core.ts, new function): The declared return type is Promise<{ serviceId: string; secretName: string }[]>, but the backend actually returns all four Secret fields: { serviceId, secretName, secretData: '***', lastUpdated?: string } (confirmed at src/api_mysecrets.ts GET handler, lines ~145-155: reply.code(200).send(secretKeys.map(entry => ({ serviceId: ..., secretName: ..., secretData: '***', lastUpdated: ... })))).

    The current type is not wrong for the stated use case (callers only need metadata), but it is factually incomplete — TypeScript will silently drop secretData and lastUpdated from any caller that destructures the result. If the CLI cmdSecrets list ever wants to display lastUpdated, it cannot without a type cast. Consider whether the return type should be the full Secret shape (matching what the wire actually delivers), or at minimum add secretData: string since the API always sends it. The same applies to getSecret's return type.

  • listSecrets sends Content-Type: application/json on a GET with no body (packages/core/src/core.ts): The header is harmless but inconsistent — other GET-only functions in core.ts (e.g. listInstances, getInstanceScaling) omit Content-Type. Not a bug, but worth aligning for consistency.


Potential Guesses Detected

None detected. Every endpoint path, HTTP method, body field name, and auth header was cross-referenced against the actual backend route handler and model definitions. No guessed values found.


Adjacent Stubs Detected

None. Swept packages/core/src/core.ts and packages/cli/src/user/cmd.ts for undefined : undefined, || undefined, TODO/FIXME/XXX, empty returns, and log-only handlers. The console.log calls in cmd.ts are all legitimate CLI output (they are the output of the CLI commands, not stubs).


Orphaned Files Detected

None. All three modified files are existing non-orphaned files. The four new exports (listSecrets, getSecret, updateSecret, deleteSecret) are all re-exported from packages/core/src/index.ts and are directly consumed in packages/cli/src/user/cmd.ts. No orphaned paths.


Impact Analysis

  • Blast radius: packages/core is the shared SDK library for all consumers of @osaas/client-core. The four new exported functions are additive — no existing exports were modified. The cmd.ts refactoring is a pure behavioral no-op (same HTTP calls, same paths, same bodies). Breaking change risk for existing consumers: none.
  • Downstream consumers: Any downstream project using @osaas/client-core will see the four new functions appear in their type definitions on the next version bump. No coordination needed — additive only.
  • Coordinated changes needed: None identified. The backend endpoints already exist (PR body confirms deploy-manager already supports these). No ConfigMap, env var, or deploy-order dependency.
  • Breaking change risk: Low. All changes are additive. The only subtle risk is that the under-typed listSecrets return type could mislead future callers into believing secretData and lastUpdated are not returned by the API.

Risks Not Tested

  • Runtime behavior of createFetch when the backend returns a non-2xx response (e.g. 400 with { reason: string }) for the new functions — error propagation not verifiable from diff alone.
  • Whether removing the inline createFetch call for the POST (saveSecret) path in cmdSecrets create actually preserved backward compatibility — the old inline code sent { secretName, secretData } while saveSecret sends { secretName: name, secretData: value } using the param named name instead of secretName at call site. This is fine at runtime (the body field is secretName in both cases) but worth a quick smoke-test of osaas secrets create after merge.
  • getSecret's .find() match is case-sensitive — no verification that the backend stores and returns secretName in a consistent case. If the backend ever normalizes case, the find will silently return undefined. Not actionable from this diff, but worth noting.

Research Suggestions

  • Before merging: Consider widening the listSecrets (and getSecret) return type to match the actual backend response shape. At minimum, add secretData: string since the API always sends it (as '***'). This prevents future callers from needing casts and makes the type a truthful reflection of what's on the wire. The backend model is Secret = { serviceId, secretName, secretData, lastUpdated? } — the SDK type could simply re-export or reference that same shape.

Reviewed by PR Research Validator. This review checks whether code is based on verified sources rather than assumptions.

@birme
birme merged commit 6e6351e into main Jun 3, 2026
1 check passed
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.

1 participant