dohertj2/scadalink-design
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(deployment-manager): resolve DeploymentManager-013 — document SMTP-credential artifact handling as a conscious accepted design decision

Browse Source
This commit is contained in:
Joseph Doherty
2026-05-16 23:49:25 -04:00
parent a2f6c1b9b2
commit c583598888
2 changed files with 37 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
code-reviews/DeploymentManager/findings.md
View File

@@ -8,7 +8,7 @@
| Last reviewed | 2026-05-16 |
| Reviewer | claude-agent |
| Commit reviewed | `9c60592` |
| Open findings | 1 |
| Open findings | 0 |
## Summary
@@ -608,7 +608,7 @@ the call hung the full 30 s and threw `AskTimeoutException`).
|--|--|
| Severity | Low |
| Category | Security |
| Status | Open |
| Status | Resolved |
| Location | `src/ScadaLink.DeploymentManager/ArtifactDeploymentService.cs:108-111` |
**Description**
@@ -655,9 +655,21 @@ hardening item, not an active leak.
**Resolution**
_Unresolved — see Verification above. Left Open: requires cross-module
cooperation (Communication, SiteRuntime, Commons) and a key-management design
decision; out of scope for the DeploymentManager module._
Resolved 2026-05-16 (commit `<pending>`). Re-verification confirmed the
DeploymentManager code is clean: `ArtifactDeploymentService` maps
`SmtpConfiguration.Credentials` into the artifact (which the design mandates —
SMTP configuration is a deployable artifact) and never logs the credential.
The finding's substantive ask — "at minimum this should be a conscious,
documented decision" — is now satisfied: a **"Secret handling in artifacts"**
subsection was added to `docs/requirements/Component-DeploymentManager.md`
recording the accepted design decision and its controls — TLS-protected
inter-cluster transport in transit, no credential values in logs, and an
explicit statement that at-rest encryption of the credential field on site
SQLite is not currently applied (accepted given the transport protection and
trust boundary) with payload-field encryption noted as a possible future
hardening item requiring a key-management scheme. No code change was warranted;
the residual encryption item is a documented, deliberately-deferred hardening
option rather than an open defect.
### DeploymentManager-014 — Dead `CreateCommand` helper in artifact tests