Conversation
dd5a251
There was a problem hiding this comment.
Pull request overview
This PR adds validation to prevent empty authentication keys from being submitted, along with GPG key verification on the backend. It also updates the key input from a single-line input to a textarea and bumps several frontend dependencies.
Changes:
- Adds frontend validation for minimum key length and switches the key input from
FormInputtoFormTextareafor better UX with long keys. - Implements server-side GPG key verification using the
pgpcrate, replacing a previousFIXMEcomment. - Bumps multiple frontend dependency versions in
package.json.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
web/src/shared/defguard-ui |
Updates the defguard-ui subproject commit reference. |
web/src/pages/.../AddAuthenticationKeyForm.tsx |
Switches key input to textarea, adds .min() validations, and adjusts regex for trimming trailing newlines. |
web/package.json |
Bumps several dependency versions. |
crates/defguard_core/src/handlers/ssh_authorized_keys.rs |
Adds GPG key verification using SignedPublicKey::from_string. |
Files not reviewed (1)
- web/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }) | ||
| .trim(), | ||
| .trim() | ||
| .min(1, LL.form.error.required()) |
There was a problem hiding this comment.
The .min(1, LL.form.error.required()) check is redundant because .min(50, ...) on the next line already guarantees the string has at least 50 characters (which implies it's non-empty). You can remove the .min(1, ...) call and rely solely on .min(50, LL.form.error.minimumLength()) — the required_error on the string() call above already handles the truly-empty / missing case.
| .min(1, LL.form.error.required()) |
| if (user) { | ||
| mutate({ | ||
| key: trimmed.keyValue.replace(/\r?\n|\r/g, ''), | ||
| key: trimmed.keyValue.replace(/[\r\n]+$/, ''), |
There was a problem hiding this comment.
The original regex /\r?\n|\r/g stripped all internal newlines/carriage-returns from the key value. The new regex /[\r\n]+$/ only removes trailing newline characters. For SSH keys (which should be a single line), internal newlines from a user pasting a wrapped key will now be preserved and sent to the server, likely causing SSH key validation to fail. Consider keeping internal newline stripping (e.g. /[\r\n]+/g) or at least stripping internal newlines for SSH keys specifically.
No description provided.