feat(api): support encrypted array start inputs#1944
Merged
Conversation
Contributor
WalkthroughAdds support to start encrypted arrays with either a decryption password or a keyfile, including validation, keyfile decoding/writing, emcmd integration, expanded logging redaction, and tests for success and failure scenarios. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant GraphQL as "GraphQL API"
participant Service as "ArrayService"
participant FS as "FileSystem"
participant Emcmd as "emcmd"
Client->>GraphQL: updateArrayState(START, decryptionPassword|decryptionKeyfile)
GraphQL->>Service: updateArrayState(desiredState, decryption inputs)
alt decryptionPassword provided
Service->>Service: validate printable ASCII
Service->>Service: base64-encode password
Service->>Emcmd: execute(command with luksKey=<encoded>)
else decryptionKeyfile provided
Service->>Service: decode data: URL or base64 payload
Service->>FS: mkdir(keyfile directory)
Service->>FS: writeFile(path, decoded, mode=0o600)
Service->>Emcmd: execute(command with luksKeyfile=<path>)
end
Emcmd-->>Service: execution result
Service-->>GraphQL: updated UnraidArray
GraphQL-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64208ae29a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1944 +/- ##
==========================================
+ Coverage 51.51% 51.60% +0.09%
==========================================
Files 1025 1025
Lines 70460 70597 +137
Branches 7793 7836 +43
==========================================
+ Hits 36297 36433 +136
- Misses 34040 34041 +1
Partials 123 123 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/src/unraid-api/graph/resolvers/array/array.service.ts (1)
153-176:⚠️ Potential issue | 🔴 CriticalDon’t acquire
pendingStatebefore the decryption preflight can throw.
pendingStateis set before the mutual-exclusion check, password encoding, and keyfile write, but thefinallythat clears it only starts aroundemcmd(). If any of those earlier steps fail, this singleton service stays stuck in “Array state is still being updated” for every laterupdateArrayState()call.🔧 Suggested fix
- // Set lock then start/stop array - this.pendingState = newPendingState; const command: Record<string, string> = { [`cmd${capitalCase(desiredState)}`]: capitalCase(desiredState), startState: constantCase(startState), }; if (decryptionPassword && decryptionKeyfile) { throw new BadRequestException( new AppError('Provide either a decryption password or a decryption keyfile, not both.') ); } - if (desiredState === ArrayStateInputState.START && decryptionPassword) { - command.luksKey = this.encodeDecryptionPassword(decryptionPassword); - } - - if (desiredState === ArrayStateInputState.START && decryptionKeyfile) { - await this.writeDecryptionKeyfile(decryptionKeyfile); - } - try { + this.pendingState = newPendingState; + + if (desiredState === ArrayStateInputState.START && decryptionPassword) { + command.luksKey = this.encodeDecryptionPassword(decryptionPassword); + } + + if (desiredState === ArrayStateInputState.START && decryptionKeyfile) { + await this.writeDecryptionKeyfile(decryptionKeyfile); + } + await emcmd(command); } finally { this.pendingState = null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/array/array.service.ts` around lines 153 - 176, pendingState is set too early (this.pendingState = newPendingState) before the mutual-exclusion check and decryption preflight (the if that throws, encodeDecryptionPassword, and writeDecryptionKeyfile), so an exception there leaves the singleton stuck; move the assignment of pendingState (newPendingState) to immediately before calling emcmd (i.e., after the mutual-exclusion check, after calling encodeDecryptionPassword and writeDecryptionKeyfile), keep the try { await emcmd(command); } finally { this.pendingState = null; } around emcmd so cleanup still occurs, and ensure no early return/throw happens before pendingState is set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/src/unraid-api/graph/resolvers/array/array.model.ts`:
- Around line 201-218: The GraphQL input type ArrayStateInput was updated to
include decryptionPassword and decryptionKeyfile but the generated client types
are stale; regenerate the GraphQL artifacts so the generated types include these
new fields. Run your GraphQL codegen (the project’s generation script / codegen
config) to re-generate the client files (the generated graphql.ts artifacts used
by the API and web clients), then verify ArrayStateInput in the generated types
includes decryptionPassword and decryptionKeyfile and adjust any call sites
using that input if needed.
In `@api/src/unraid-api/graph/resolvers/array/array.service.ts`:
- Around line 47-59: The data URL branch using dataUrlMatch/meta/payload
currently calls Buffer.from(payload, 'base64') without validating the base64
payload; reuse the same base64 validation used for raw payloads (the
/^[A-Za-z0-9+/]+={0,2}$/ style check used where raw payloads are decoded) to
verify payload before decoding, and if it fails throw the existing
BadRequestException(new AppError(...)); apply this validation path in the
;base64 branch prior to calling Buffer.from(payload, 'base64') so malformed
data:...;base64,... inputs are rejected instead of producing silent corruption.
---
Outside diff comments:
In `@api/src/unraid-api/graph/resolvers/array/array.service.ts`:
- Around line 153-176: pendingState is set too early (this.pendingState =
newPendingState) before the mutual-exclusion check and decryption preflight (the
if that throws, encodeDecryptionPassword, and writeDecryptionKeyfile), so an
exception there leaves the singleton stuck; move the assignment of pendingState
(newPendingState) to immediately before calling emcmd (i.e., after the
mutual-exclusion check, after calling encodeDecryptionPassword and
writeDecryptionKeyfile), keep the try { await emcmd(command); } finally {
this.pendingState = null; } around emcmd so cleanup still occurs, and ensure no
early return/throw happens before pendingState is set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d1618bda-56d1-4d9d-9261-cb5b94d74de2
📒 Files selected for processing (5)
api/src/core/log.tsapi/src/core/utils/clients/emcmd.tsapi/src/unraid-api/graph/resolvers/array/array.model.tsapi/src/unraid-api/graph/resolvers/array/array.service.spec.tsapi/src/unraid-api/graph/resolvers/array/array.service.ts
Contributor
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/src/unraid-api/graph/resolvers/array/array.service.ts (1)
166-172: Consider validating decryption inputs aren't provided with STOP.If a caller mistakenly provides
decryptionPasswordordecryptionKeyfilewhendesiredStateisSTOP, the inputs are silently ignored. Throwing an explicit error would provide clearer feedback:🔧 Suggested validation
command.luksKey = this.encodeDecryptionPassword(decryptionPassword); } if (desiredState === ArrayStateInputState.START && decryptionKeyfile) { await this.writeDecryptionKeyfile(decryptionKeyfile); } + if (desiredState === ArrayStateInputState.STOP && (decryptionPassword || decryptionKeyfile)) { + throw new BadRequestException( + new AppError('Decryption credentials are only applicable when starting the array.') + ); + } + this.pendingState = newPendingState;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/unraid-api/graph/resolvers/array/array.service.ts` around lines 166 - 172, When desiredState === ArrayStateInputState.STOP and either decryptionPassword or decryptionKeyfile is provided, add explicit validation to throw an error instead of silently ignoring them: check (desiredState === ArrayStateInputState.STOP && (decryptionPassword || decryptionKeyfile)) and throw a descriptive Error (or a domain-specific error) indicating decryption inputs are only valid for START; keep the existing calls to encodeDecryptionPassword and writeDecryptionKeyfile for the START branch (methods referenced: encodeDecryptionPassword, writeDecryptionKeyfile, and the desiredState/ArrayStateInputState checks) so this validation runs before those START-specific blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/src/unraid-api/graph/resolvers/array/array.service.ts`:
- Around line 166-172: When desiredState === ArrayStateInputState.STOP and
either decryptionPassword or decryptionKeyfile is provided, add explicit
validation to throw an error instead of silently ignoring them: check
(desiredState === ArrayStateInputState.STOP && (decryptionPassword ||
decryptionKeyfile)) and throw a descriptive Error (or a domain-specific error)
indicating decryption inputs are only valid for START; keep the existing calls
to encodeDecryptionPassword and writeDecryptionKeyfile for the START branch
(methods referenced: encodeDecryptionPassword, writeDecryptionKeyfile, and the
desiredState/ArrayStateInputState checks) so this validation runs before those
START-specific blocks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8f9589a3-71c5-43d5-a1ef-f29b61f1e0a0
📒 Files selected for processing (2)
api/src/unraid-api/graph/resolvers/array/array.service.spec.tsapi/src/unraid-api/graph/resolvers/array/array.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- api/src/unraid-api/graph/resolvers/array/array.service.spec.ts
elibosley
pushed a commit
that referenced
this pull request
Mar 23, 2026
🤖 I have created a release *beep* *boop* --- ## [4.31.0](v4.30.1...v4.31.0) (2026-03-23) ### Features * **api:** support encrypted array start inputs ([#1944](#1944)) ([018a8d5](018a8d5)) * **onboarding:** add shared loading states ([#1945](#1945)) ([776c8cc](776c8cc)) * Serverside state for onboarding display ([#1936](#1936)) ([682d51c](682d51c)) ### Bug Fixes * **api:** reconcile emhttp state without spinning disks ([#1946](#1946)) ([d3e0b95](d3e0b95)) * **onboarding:** auto-open incomplete onboarding on 7.3+ ([#1940](#1940)) ([f0241a8](f0241a8)) * **onboarding:** replace internal boot native selects ([#1942](#1942)) ([d6ea032](d6ea032)) * preserve onboarding resume state on reload ([#1941](#1941)) ([91f7fe9](91f7fe9)) * recover VM availability after reconnect ([#1947](#1947)) ([e064de7](e064de7)) * Unify callback server payloads ([#1938](#1938)) ([f58fcc0](f58fcc0)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
pnpm --filter ./api test src/unraid-api/graph/resolvers/array/array.service.spec.tspnpm --filter ./api type-checkCloses #1943
Summary by CodeRabbit
New Features
Improvements
Tests