Reject SSH key ciphers with null or empty required fields

[xhon-pelushi]

Summary

• Validate SSH key (type: 5) payloads before save.
• Require non-empty string values for privateKey, publicKey, and keyFingerprint.
• Reject null/empty/missing fields with an error instead of accepting the write and dropping sshKey on read-back.

Fixes #7514

Test plan

• cargo test --profile ci --features sqlite ssh_key_validation_tests
• POST/PUT a type-5 cipher with any required sshKey field set to null returns an error.
• POST/PUT with non-empty placeholder strings for all three fields succeeds and round-trips.
• Existing non-SSH cipher types are unchanged.

[BlackDex]

I think the validation function can be a bit optimized.

The tests i find a bit too much t.b.h.
It might also be an option to just create one test function and have a vec of cases to check and then just run those?

Something like this maybe?

    let cases = [
        // missing field
        (json!({"publicKey":"pub","keyFingerprint":"fp"}), false),
        // null field
        (json!({"privateKey":null,"publicKey":"pub","keyFingerprint":"fp"}), false),
    ]

And then loop over those cases and use the false or true as an assert outcome?
That would make it easier to add other tests if needed in the future and thinking about different function names. The failed test is shown in detail if i'm correct, so we should see what would failed (but i might be wrong here).

[BlackDex]

This function is located at a wrong place, it cuts into the comment of an other function.

I also think it could be a bit more optimized maybe?

Suggested change

	/// Ensure SSH key type-data has the required non-empty string fields.
	fn validate_ssh_key_data(type_data: &Value) -> EmptyResult {
	for field in ["privateKey", "publicKey", "keyFingerprint"] {
	match type_data.get(field).and_then(Value::as_str) {
	Some(value) if !value.is_empty() => {}
	_ => err!(format!("SSH key field '{field}' must be a non-empty string")),
	}
	}
	Ok(())
	}
	fn validate_ssh_key_data(type_data: &Value) -> EmptyResult {
	for field in ["privateKey", "publicKey", "keyFingerprint"] {
	type_data
	.get(field)
	.and_then(Value::as_str)
	.filter(|v| !v.is_empty())
	.ok_or_else(|| err!(format!("SSH Key field '{field}' is required!")))?;
	}
	}

[xhon-pelushi]

Thanks — all three addressed in 11f4175.

Placement. You were right, and it was worse than misplaced: the function had landed between the third and fourth lines of the enforce_personal_ownership_policy doc comment, so /// (that were created before the policy was applicable to the user) was orphaned below it. Moved below that function, comment is one block again.

The function. I went with the idiom already in the tree rather than my own — Cipher::to_json checks these same three fields at src/db/models/cipher.rs:

if self.atype == 5
    && (type_data_json["keyFingerprint"].as_str().is_none_or(str::is_empty)
        || type_data_json["privateKey"].as_str().is_none_or(str::is_empty)
        || type_data_json["publicKey"].as_str().is_none_or(str::is_empty))

so this now reads:

fn validate_ssh_key_data(type_data: &Value) -> EmptyResult {
    for field in ["privateKey", "publicKey", "keyFingerprint"] {
        if type_data[field].as_str().is_none_or(str::is_empty) {
            err!(format!("SSH key field '{field}' must be a non-empty string"))
        }
    }
    Ok(())
}

I added a doc comment noting how the two relate — to_json discards the type-data of ciphers already stored with bad fields, this stops them being written in the first place.

One note on your suggested snippet: err! expands to return Err(...) (src/error.rs), so inside .ok_or_else(|| err!(...)) it would return from the closure rather than the function and wouldn't type-check. Same shape otherwise.

Tests. Collapsed to one table as you suggested, and added two cases the originals missed — a non-string field, and type-data with no fields at all:

let cases = [
    ("all fields present", json!({...}), true),
    ("null private key",   json!({...}), false),
    ...
];
for (case, type_data, expected_ok) in cases {
    assert_eq!(validate_ssh_key_data(&type_data).is_ok(), expected_ok, "case: {case}");
}

To your question about failure output — with a bare assert! you'd only get the line number, so I gave each case a label that assert_eq! prints. A failure now reads case: empty public key.

Verified locally on the pinned 1.97.1 toolchain: cargo test --features sqlite 30 passed / 0 failed, cargo fmt --check clean.

Unrelated, but you may want to know: cargo clippy --features sqlite --all-targets -- -D warnings fails on src/api/admin.rs:908-910 with "used assert! with an equality comparison" —

assert!(web_vault_compare("2025.12.0", "2025.12.1") == -1);

Nothing to do with this PR (it only touches ciphers.rs), but it does mean that clippy job should be red on main too.