mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-20 12:30:38 +09:00
exe.dev config UX: advanced-options disclosure, form-default fix, SSH key handling (PAPA-407) (#7025)
## Thinking Path
> - Paperclip orchestrates AI agents and provisions sandboxed execution
environments for them; one of those provisioners is the exe.dev plugin,
which runs each agent inside a long-lived VM reached over SSH.
> - The instance-config form for that plugin is rendered generically by
`JsonSchemaForm` from the plugin's `instanceConfigSchema`, so any UX
problem with the form is split between the shared form component and the
plugin's schema/runtime code.
> - Users coming in cold hit a 12-field flat config they couldn't reason
about (PAPA-407), a form that silently submitted `cpu: 0` for untouched
optional fields (PAPA-407 root cause), a `sshPrivateKey` textarea that
truncated RSA-4096 keys at 4096 chars (PAPA-449), a save flow that
accepted clearly-malformed keys and only blew up at lease time with raw
SSH stderr (PAPA-450, PAPA-451), and a manifest that didn't distinguish
"essential" from "advanced" knobs (PAPA-410 / PAPA-411 — duplicate
sub-issues with identical scope; PAPA-418 reconciliation kept PAPA-410
canonical).
> - These problems all point at the same surface (exe.dev sandbox
config) and are tightly coupled in code — PAPA-449/450/451 patch fields
that PAPA-410/411 introduce — so they get reviewed together.
> - This pull request lands the shared-form changes (advanced-options
disclosure, optional-scalar defaults) and the exe.dev-specific changes
(manifest restructure, longer `maxLength`, stderr translation, save-time
key validation) as five focused commits stacked on `master`.
> - The benefit is a config form that defaults to the two fields a new
user actually needs (API key + SSH private key) with a collapsible
disclosure for the rest, no silent truncation or zero-default
submissions, and SSH key problems surfaced at save time with actionable
messages instead of cryptic post-provision failures.
## What Changed
- **JsonSchemaForm advanced-options disclosure** (PAPA-410, PAPA-411 —
same scope, see note above): adds `x-paperclip-advanced` /
`x-paperclip-group` schema annotations and renders flagged fields behind
a collapsible "Advanced options" disclosure that auto-opens when a
hidden field has a validation error. Exe.dev manifest is restructured to
use the new annotations, so essentials (`apiKey`, `sshPrivateKey`) show
by default while the long tail of optional knobs is grouped under "SSH
access" / "VM resources" / "More options" headings.
- **Omit optional scalar defaults** (PAPA-407): `getDefaultForSchema` no
longer materialises `0` / `""` for optional
`number`/`integer`/`string`/`secret-ref` fields without an explicit
`default`. Object recursion drops properties whose default is
`undefined`. Fields that declare a `default` (e.g. `sshPort: 22`) still
round-trip. Adds a regression test against `getDefaultValues`.
- **Raise `sshPrivateKey` `maxLength`** (PAPA-449): bumps the exe.dev
manifest cap from 4096 to 8192 so RSA-4096 OpenSSH private keys (which
can exceed 4 KB with comments/metadata) aren't silently truncated at
submit.
- **Translate `invalid format` SSH stderr** (PAPA-450):
`formatSshFailure` now recognises `Load key … invalid format` in
combined stderr/stdout and returns a specific message naming the
key-format problem ("isn't an OpenSSH/PEM private key — confirm the
secret starts with `-----BEGIN … PRIVATE KEY-----` and isn't the `.pub`
or a PuTTY `.ppk` export") instead of dumping the raw stderr.
- **Save-time SSH key validation** (PAPA-451):
`onEnvironmentValidateConfig` inline-parses `sshPrivateKey` and rejects
common failure modes — pasted public keys, PuTTY `.ppk` format, missing
`-----END-----` footer, non-base64 body — so the form surfaces an inline
error before any VM is provisioned. Secret-ref bindings (UUIDs) are
still passed through unchanged.
## Verification
CI gates (`pnpm typecheck`, `pnpm test`, the targeted vitest suites
below) all pass.
Run locally:
```bash
# Shared form
pnpm --filter @paperclipai/ui exec vitest run src/components/JsonSchemaForm
# 9 tests pass — includes the new "omits optional scalar fields" regression
# and the three advanced-options-disclosure tests.
# exe.dev plugin
cd packages/plugins/sandbox-providers/exe-dev && pnpm test
# 32 tests pass — includes the new sshPrivateKey-validation cases
# and the new "invalid format" stderr-translation case.
```
Manual smoke (after reinstalling the plugin so the DB manifest
refreshes):
1. Open the exe.dev environment config page. **Default view shows API
Key + SSH Private Key only**, with an "Advanced options" disclosure for
everything else (PAPA-410 / PAPA-411).
2. Paste a `.pub` file's contents into SSH Private Key, click Save.
**Inline error** rejecting the wrong-format key (PAPA-451).
3. Re-paste a valid OpenSSH/PEM private key longer than 4096 bytes —
saves cleanly (PAPA-449).
4. Save the form with everything optional left blank — server no longer
rejects with `"cpu must be greater than 0 when provided"` (PAPA-407).
5. Force a bad key through via a stored secret-ref binding and lease a
VM — failure message names the key-format problem instead of dumping raw
SSH stderr (PAPA-450).
## Risks
- **PAPA-410 / PAPA-411 manifest restructure** is the largest surface
here. Schemas using `x-paperclip-*` extensions are forward-compatible
with stricter JSON Schema validators (extensions are ignored by
default), and the form gracefully renders a flat layout when no field
opts in.
- **PAPA-407** changes form-default behaviour: optional scalar fields
that previously round-tripped as `""` / `0` will now be `undefined` and
absent from the submitted payload. Downstream consumers that expected
the empty-string/zero shape need to treat the field as optional.
Spot-checked the existing exe.dev driver — it already uses
`parseOptionalString` / `parseOptionalInteger`, which treat missing
fields as `null` rather than `0`/`""`.
- **PAPA-451** adds a save-time check, so a
previously-saved-but-malformed `sshPrivateKey` raw value will now fail
to re-save. Bound secret-refs are unaffected, matching how the user
reaches the bad-key state today (via the secrets picker).
- **PAPA-449** simply raises a cap; no semantic risk.
- **PAPA-450** only kicks in on the "invalid format" code path; existing
onboarding-marker branch is untouched.
## Model Used
- Provider: Anthropic
- Model: Claude Opus 4.7 (`claude-opus-4-7`)
- Capabilities used: code reading, code editing, test execution, git/PR
mechanics, Paperclip API for issue coordination
## Checklist
- [x] PR body sections present (Thinking Path, What Changed,
Verification, Risks, Model Used, Checklist)
- [x] Unit tests added for the new behaviours (JsonSchemaForm
default-value omission + advanced disclosure; exe.dev plugin validation
+ stderr translation)
- [x] Existing tests still pass locally (`vitest run` on both packages)
- [x] No raw secrets, IP addresses, or machine-local config in commits
or PR body
- [x] Commits are atomic per linked issue (PAPA-410 / PAPA-411,
PAPA-407, PAPA-449, PAPA-450, PAPA-451)
- [x] Branch is up-to-date with `origin/master`
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
8014445b23
commit
aea35fe695
8 changed files with 694 additions and 128 deletions
|
|
@ -68,6 +68,8 @@ const SSH_SIGKILL_GRACE_MS = 250;
|
|||
const MAX_VM_RECORD_DEPTH = 4;
|
||||
const EXE_DEV_SSH_ONBOARDING_MARKER = "Please complete registration by running: ssh exe.dev";
|
||||
const EXE_DEV_SSH_EMAIL_PROMPT = "Please enter your email address:";
|
||||
const EXE_DEV_SSH_INVALID_KEY_FORMAT = /Load key [^\n]*invalid format/i;
|
||||
const UUID_SECRET_REF_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
|
||||
|
||||
// exe.dev's `--setup-script` runs at VM init as the unprivileged `exedev` user, which
|
||||
// has passwordless sudo. The Paperclip sandbox callback bridge is a Node script, so
|
||||
|
|
@ -139,6 +141,74 @@ function isValidUrl(value: string): boolean {
|
|||
}
|
||||
}
|
||||
|
||||
function isSecretRef(value: string): boolean {
|
||||
return UUID_SECRET_REF_RE.test(value);
|
||||
}
|
||||
|
||||
// Catch the SSH-key paste failure modes we've seen in the wild (wrong file,
|
||||
// PPK export, truncated paste) before the user pays the cost of provisioning a
|
||||
// VM and getting a cryptic SSH error. Inline parse — no `ssh-keygen` dependency
|
||||
// — so this also works on hosts where openssh-client isn't installed.
|
||||
export function validateSshPrivateKey(rawKey: string): string | null {
|
||||
const trimmed = rawKey.trim();
|
||||
if (!trimmed) return null;
|
||||
|
||||
if (/^PuTTY-User-Key-File-\d/m.test(trimmed)) {
|
||||
return "sshPrivateKey looks like a PuTTY .ppk file. Convert it to OpenSSH format (PuTTYgen → Conversions → Export OpenSSH key) and paste the resulting PEM.";
|
||||
}
|
||||
|
||||
if (
|
||||
/^(?:ssh-(?:rsa|dss|ed25519)|ecdsa-sha2-[a-z0-9-]+|sk-(?:ssh-ed25519|ecdsa-sha2-[a-z0-9-]+)@openssh\.com)\s+\S/.test(
|
||||
trimmed,
|
||||
)
|
||||
) {
|
||||
return "sshPrivateKey looks like a PUBLIC key. Paste the matching private key (the file without the .pub extension).";
|
||||
}
|
||||
|
||||
const headerMatch = trimmed.match(/^-----BEGIN ([A-Z0-9 ]*)PRIVATE KEY-----/m);
|
||||
if (!headerMatch) {
|
||||
return "sshPrivateKey must be a PEM-encoded private key starting with a line like '-----BEGIN OPENSSH PRIVATE KEY-----'.";
|
||||
}
|
||||
|
||||
const footerMatch = trimmed.match(/^-----END ([A-Z0-9 ]*)PRIVATE KEY-----\s*$/m);
|
||||
if (!footerMatch) {
|
||||
return "sshPrivateKey is missing its '-----END … PRIVATE KEY-----' footer. Make sure you copied the whole file, including the final line.";
|
||||
}
|
||||
|
||||
const headerLabel = headerMatch[1].trim();
|
||||
const footerLabel = footerMatch[1].trim();
|
||||
if (headerLabel !== footerLabel) {
|
||||
return `sshPrivateKey header/footer mismatch (BEGIN ${headerLabel || "(none)"} vs END ${footerLabel || "(none)"}). The file is likely truncated or two keys are concatenated.`;
|
||||
}
|
||||
|
||||
const headerLineEnd = trimmed.indexOf("\n", headerMatch.index ?? 0);
|
||||
const footerStart = trimmed.lastIndexOf(footerMatch[0]);
|
||||
if (headerLineEnd < 0 || footerStart <= headerLineEnd) {
|
||||
return "sshPrivateKey appears to be empty between its BEGIN and END markers.";
|
||||
}
|
||||
|
||||
const bodyLines = trimmed
|
||||
.slice(headerLineEnd + 1, footerStart)
|
||||
.split(/\r?\n/)
|
||||
.map((line) => line.trim())
|
||||
.filter((line) => line.length > 0);
|
||||
if (bodyLines.length === 0) {
|
||||
return "sshPrivateKey appears to be empty between its BEGIN and END markers.";
|
||||
}
|
||||
|
||||
// PEM bodies are base64 lines, optionally preceded by `Header: value` lines
|
||||
// on encrypted PKCS#1 keys (`Proc-Type:`, `DEK-Info:`).
|
||||
const base64Line = /^[A-Za-z0-9+/=]+$/;
|
||||
const pemHeaderLine = /^[A-Za-z][A-Za-z0-9-]*:\s.+$/;
|
||||
for (const line of bodyLines) {
|
||||
if (!base64Line.test(line) && !pemHeaderLine.test(line)) {
|
||||
return "sshPrivateKey body contains non-base64 characters. The key may have been corrupted by line-wrapping or copy-paste.";
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
function normalizeApiUrl(value: string | null): string {
|
||||
if (!value) return DEFAULT_API_URL;
|
||||
const trimmed = value.trim();
|
||||
|
|
@ -498,6 +568,13 @@ function formatSshFailure(
|
|||
].join(" ");
|
||||
}
|
||||
|
||||
if (EXE_DEV_SSH_INVALID_KEY_FORMAT.test(combinedOutput)) {
|
||||
return [
|
||||
`Failed to ${action} exe.dev VM ${vmName}: the configured SSH private key isn't an OpenSSH-format private key.`,
|
||||
"Confirm the secret starts with `-----BEGIN … PRIVATE KEY-----` and isn't the `.pub` file or a PuTTY `.ppk` export.",
|
||||
].join(" ");
|
||||
}
|
||||
|
||||
return `Failed to ${action} exe.dev VM ${vmName}: ${result.stderr.trim() || result.stdout.trim() || "unknown error"}`;
|
||||
}
|
||||
|
||||
|
|
@ -686,6 +763,10 @@ const plugin = definePlugin({
|
|||
) {
|
||||
errors.push("strictHostKeyChecking cannot be empty.");
|
||||
}
|
||||
if (config.sshPrivateKey && !isSecretRef(config.sshPrivateKey)) {
|
||||
const sshKeyError = validateSshPrivateKey(config.sshPrivateKey);
|
||||
if (sshKeyError) errors.push(sshKeyError);
|
||||
}
|
||||
|
||||
warnings.push(
|
||||
"The Paperclip host must have SSH access to the created exe.dev VM, and its SSH key must be registered with exe.dev. The API token only covers provisioning.",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue