[irvingouj@Devolutions (irvingoujAtDevolution)]

Adds SecretStore, a place for secrets that must stay in memory and never hit the disk, like the API key of an AI analysis (#2044).

• keyed by a caller-picked SecretId, not a JTI, so it stays out of ProvisioningStore, which is about sessions
• encrypted with the same in-memory master key as injected credentials, and decrypted into a zeroizing SecretString only on read
• a new secret under the same ID replaces the old one
• gone when the caller removes it, or at its expiry through the store's own cleanup task
• opaque: the caller decides what the secret holds

credential gets encrypt_secret / decrypt_secret over the existing master key, plus an EncryptedSecret alias for EncryptedPassword.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Caller-controlled timestamps can bypass task-secret expiry enforcement.

1 open finding

• Use current time internally when enforcing task secret expiry · New

What changed in this PR

Adds encrypted, in-memory storage for provisioner task secrets.

Changes:

• Adds task-secret insertion, retrieval, removal, expiry, and cleanup.
• Exposes credential encryption helpers for arbitrary secrets.
• Adds lifecycle and isolation tests.

File	Description
devolutions-gateway/​src/​provisioning.rs	Implements the task-secret store and tests.
devolutions-gateway/​src/​credential/​mod.rs	Adds secret encryption and decryption helpers.

🧠 Review effort: Balanced

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The scoped changes reuse established encryption and cleanup mechanisms, with no unresolved findings.

No open issues 🎉

1 resolved since last review

• Use current time internally when enforcing task secret expiry

_{Review effort: Balanced}

[Benoît Cortier (CBenoit)]

Thank you Irving.

• I have doubts about this change being in ProvisioningStore at all, because ProvisioningStore was intended to be for keyed by session ID (JTI), and this PR is adding task ID keyed data.
• Also, I think the new code is over-fitting the specific use case you had in mind. It seems to me that the mechanism itself does not need to be task-specific; almost nothing in TaskSecretEntry is specific enough to be a task thing. The mechanism also sounds generally useful enough that we may use it again in the future.

Here is the AI-powered review based on these observations:

1. Should this live in ProvisioningStore at all?

credential/INTENT.md defines provisioning around a logical session: material keyed by the association-token JTI, staged then checked out, and ejected once the session can no longer connect or reconnect. Decision 2's "per-kind policy", quoted in the description, is about kinds of provisioning for the same JTI.

These secrets are keyed by a non-JTI ID, aren't tied to a session or connection, and have a caller-managed lifecycle. As task_secrets_are_independent_from_session_provisioning shows, they share no state or rules with the session halves, only the master key and the expiry cleanup. Adding them here turns ProvisioningStore from "what a session needs" into "anything kept in memory with an expiry". INTENT.md would then no longer describe everything the store does.

I'd rather put this in a small standalone store (e.g. SecretStore) that:

• reuses credential::encrypt_secret / decrypt_secret (that part is fine as is)
• has its own expiry cleanup, or shares a generic expiring-map helper with ProvisioningStore
• is owned by the #2044 caller, which already manages the task's lifecycle

If these secrets are meant to be tied to a session, please say so, and then keying by JTI here would make sense.

2. Name it for its lifecycle, not its first consumer

What this adds is generic: an opaque, in-memory-only secret, keyed by an ID, replaced on re-insert, removed explicitly or at expiry. The store never relies on anything task-specific. "Task" also collides with devolutions_gateway_task::Task, which provisioning.rs imports, and which the store's own cleanup runs as.

• insert_task_secret / task_secret / remove_task_secret → insert / get / remove on the new store (or insert_secret / secret / remove_secret if it stays here)
• TaskSecretEntry → SecretEntry
• Key it with a newtype (e.g. SecretId) rather than a bare Uuid, so callers say which ID space they're using and IDs can't be confused with JTIs.
• Nit: encrypt_secret / decrypt_secret return EncryptedPassword, so an API key ends up stored as a "password". An EncryptedSecret alias or rename would match the new entry points.

3. Keep the AI analysis out of the store's docs

The store docs should describe the policy only, for example:

/// Opaque secrets that must stay in memory only and never be written to disk.
/// Each is keyed by a caller-chosen ID, encrypted with the in-memory master key, replaced on
/// re-insert, and kept until it is removed or reaches its expiry.

Why the AI analysis needs it, and what it puts in the secret, belongs at the call site in #2044. The tests can use neutral values too ("secret-a" rather than "sk-…").

I think SecretId could be namespaced if reusing the same store later for another feature is a concern.

[irvingouj@Devolutions (irvingoujAtDevolution)]

Done in bfd3ecc. It's a standalone SecretStore now, keyed by a SecretId newtype, with insert/get/remove, its own cleanup task, and docs that only state the policy. I went with an EncryptedSecret alias instead of renaming EncryptedPassword, to keep this PR small. No namespace on SecretId for now: each user gets its own store. #2044 keeps the AI access in it now (a10bbd6).

Note

Human-tuned, LLM-assisted content.