Arch/cipher scene

[MGibson1]

📔 Objective

Provide seeder api scenes for cipher creation. To reduce API complexity, I elected to create multiple scenes rather than a single that keys off of a type.

📸 Screenshots

[github-actions]

Checkmarx One – Scan Summary & Details – 9114c542-be84-48d3-b950-bae6d08a9a3b

---

New Issues (3)

Checkmarx found the following issues in this Pull Request

#	Severity	Issue	Source File / Package	Checkmarx Insight
1		Path_Traversal	/src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs: 56	details Method at line 56 of /src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs gets dynamic data from the model element. This ... Attack Vector
2		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1592	details Method at line 1592 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector
3		CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 1419	details Method at line 1419 of /src/Api/Vault/Controllers/CiphersController.cs gets a parameter from a user request from id. This parameter value flows ... Attack Vector

---

Fixed Issues (2)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	CSRF	/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 105
	CSRF	/src/Api/Vault/Controllers/CiphersController.cs: 293

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.68%. Comparing base (fa5fde5) to head (62940a7).
⚠️ Report is 66 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7241      +/-   ##
==========================================
+ Coverage   56.87%   57.68%   +0.80%     
==========================================
  Files        2028     2035       +7     
  Lines       88827    89645     +818     
  Branches     7917     7993      +76     
==========================================
+ Hits        50523    51709    +1186     
+ Misses      36472    36072     -400     
- Partials     1832     1864      +32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Overall Assessment: REQUEST CHANGES

This PR adds five new cipher scene types (Card, Identity, Login, SecureNote, SSH Key) for the Seeder API, adds reprompt and deleted parameters to existing cipher factories, and refactors DestroyBatchScenesCommand from parallel to sequential execution. The new scenes follow established patterns consistently. One correctness issue was found where the deleted flag on login ciphers is silently ignored due to a missing propagation step.

Code Review Details

• ⚠️ : DeletedDate set on CipherViewDto but never copied to the Cipher entity by CreateEntity, so deleted: true has no effect
  • util/Seeder/Factories/LoginCipherSeeder.cs:35

Prior unresolved findings (still applicable):

• ValueTuple Fields property on UserLoginCipherScene.Request will not deserialize correctly from JSON (thread on UserLoginCipherScene.cs:25)

[theMickster]

Two minor but important things I think we should change now because we aren't likely to come back to these scenes for any tech debt refactor for a long while.

[claude]

🔵 [SUGGESTION] C# value tuples (ValueTuple) don't round-trip through System.Text.Json with named elements — at runtime the names are Item1, Item2, Item3, not name, value, type. Since SceneExecutor deserializes requests via JsonSerializer.Deserialize, callers would need to send {"Item1": ..., "Item2": ..., "Item3": ...} for this to work, which is unintuitive.

Consider using FieldViewDto (or a dedicated DTO) instead of the tuple so the JSON contract is explicit and matches the field names callers would expect:

Suggested change

	public IEnumerable<(string name, string value, int type)>? Fields { get; set; }
	public IEnumerable<FieldViewDto>? Fields { get; set; }

This would require a small adapter in SeedAsync to convert to the tuple the factory expects, or updating LoginCipherSeeder.Create to accept FieldViewDto directly.

[theMickster]

@MGibson1 Will you also try to see what might be going on with the Docker build job for the Seeder? I don't suspect that this code broke anything, but I just don't want to forget about it throwing exceptions.

[claude]

⚠️ IMPORTANT: DeletedDate is set on CipherViewDto but never propagated to the Cipher entity -- setting deleted: true silently has no effect

Details and fix

CipherEncryption.CreateEntity (in CipherEncryption.cs lines 46-57) does not copy DeletedDate from EncryptedCipherDto to the returned Cipher entity. It only sets Id, OrganizationId, UserId, Type, Data, Key, Reprompt, CreationDate, and RevisionDate.

The EncryptedCipherDto does carry DeletedDate through encryption, but it is discarded when building the final entity.

Fix in CipherEncryption.CreateEntity:

return new Cipher
{
    Id = CoreHelpers.GenerateComb(),
    OrganizationId = organizationId,
    UserId = userId,
    Type = cipherType,
    Data = dataJson,
    Key = encrypted.Key,
    Reprompt = (CipherRepromptType?)encrypted.Reprompt,
    CreationDate = DateTime.UtcNow,
    RevisionDate = DateTime.UtcNow,
    DeletedDate = encrypted.DeletedDate,
};

This requires adding a DeletedDate parameter to CreateEntity or reading it from the EncryptedCipherDto that is already passed in.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud