Skip to content

fix(dgw): address reviewer feedback on in-memory credential encryption#1690

Merged
Benoît Cortier (CBenoit) merged 3 commits intoDGW-326from
copilot/sub-pr-1689
Feb 26, 2026
Merged

fix(dgw): address reviewer feedback on in-memory credential encryption#1690
Benoît Cortier (CBenoit) merged 3 commits intoDGW-326from
copilot/sub-pr-1689

Conversation

Copy link
Contributor

Copilot AI commented Feb 26, 2026

Fixes three issues flagged during review of the in-memory credential encryption PR.

Changes

  • Typo fix (rdp_proxy.rs, rd_clean_path.rs): "an the FQDN format" → "in the FQDN format"
  • Error classification (credential/mod.rs, preflight.rs): Introduced a typed InsertError enum on CredentialStoreHandle::insert with InvalidToken (client-side bad input) and Internal (server-side failure) variants, so callers can distinguish error kinds without needing to inspect error messages or duplicate logic
pub enum InsertError {
    /// The provided token is invalid (e.g., missing or malformed JTI).
    InvalidToken(anyhow::Error),
    /// An internal error occurred (e.g., encryption failure).
    Internal(anyhow::Error),
}
  • Docs (README.md): Document libsodium as a required native build dependency under "From sources", with platform-specific install instructions (Linux/macOS/Windows)

Issue: DGW-326


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…docs

Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 26, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Copilot AI changed the title [WIP] Add ChaCha20-Poly1305 encryption for credentials store fix(dgw): address reviewer feedback on in-memory credential encryption Feb 26, 2026
Comment on lines 340 to 344
// Validate the token JTI up front so that a bad/missing JTI is reported as
// InvalidParams (client error) rather than InternalServerError.
crate::token::extract_jti(&token)
.map_err(|e| PreflightError::new(PreflightAlertStatus::InvalidParams, format!("{e:#}")))?;

Choose a reason for hiding this comment

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

Copilot modify the insert function such that it returns an error type that allows us to distinguish between invalid parameters and internal errors (eg; using an enum). This avoids doing the need for knowing the internals and doing the job twice as precheck (that's an anti pattern)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in commit 1da4403. Added a InsertError enum with InvalidToken and Internal variants to credential/mod.rs. Both CredentialStore::insert and CredentialStoreHandle::insert now return Result<_, InsertError>, with JTI extraction failures mapped to InvalidToken and encryption failures to Internal. The pre-check in preflight.rs is replaced by a match on the error variant.

Co-authored-by: CBenoit <3809077+CBenoit@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 26, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

@CBenoit Benoît Cortier (CBenoit) merged commit c10ad70 into DGW-326 Feb 26, 2026
26 of 33 checks passed
@CBenoit Benoît Cortier (CBenoit) deleted the copilot/sub-pr-1689 branch February 26, 2026 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants