From d401efabd5ae0cf15008d54e39ba815d942beb7c Mon Sep 17 00:00:00 2001 From: william-xie Date: Thu, 16 Jul 2026 13:20:02 -0700 Subject: [PATCH 1/3] fix: parent folder prefix in all compressed entries --- .sdd/ui-bundle-upload/plan.md | 2 +- .sdd/ui-bundle-upload/spec.md | 2 +- messages/ui-bundle.upload.md | 4 +-- src/commands/ui-bundle/upload.ts | 16 ++++++--- test/commands/ui-bundle/upload.test.ts | 47 ++++++++++++++++++++------ 5 files changed, 53 insertions(+), 18 deletions(-) diff --git a/.sdd/ui-bundle-upload/plan.md b/.sdd/ui-bundle-upload/plan.md index 97b755a..3ecf1f8 100644 --- a/.sdd/ui-bundle-upload/plan.md +++ b/.sdd/ui-bundle-upload/plan.md @@ -75,7 +75,7 @@ None of the remaining non-goals (301/303/304/305) require a dedicated implementa | 2.4 | Implement `bundleName` default derivation and wire it into the `deployRequest` JSON part. Reuse the already-computed `zipFilename` local (set to `${basename(bundleDir)}.zip` for `--bundle-dir` or `basename(zipFile)` for `--zip-file`), apply a case-insensitive `.zip` strip (`zipFilename.replace(/\.zip$/i, '')`), and fall back to the unstripped name if stripping leaves an empty string (a `--zip-file` literally named `.zip`/`.ZIP`): `const strippedZipFilename = zipFilename.replace(/\.zip$/i, ''); const bundleName = flags['bundle-name'] ?? (strippedZipFilename | | zipFilename);`. Pass this into `deployRequest`'s `requestedName`field:`form.append('deployRequest', JSON.stringify({ requestedName: bundleName }), { contentType: 'application/json' });`. Previously `deployRequest`was sent as an empty`{}`. | spec §2.4 (REQ-116) | | 2.5 | Implement `connection.request()` call to `POST /services/data/v62.0/connect/ui-bundle/deployments` with a `multipart/form-data` body carrying the two REQUIRED parts the contract specifies (spec §2.5): a `deployRequest` part (`application/json`, now `{requestedName: bundleName}` per Step 2.4) and a `bundle` part (binary, the zip payload from either branch of 2.3, matching the server-side `@ConnectParameter(name = "bundle", type = ParameterType.Binary, minVersion = 262)` declaration) + a `pages: flags['use-salesforce-pages']` form field. **The multipart `bundle` transport itself is the LOCKED design** — AC6 resolved per spec §2.5, confirmed by PR #118209 which also removed `contentReference`. Two caveats (spec §2.5): (a) PR #118209 is open/not-merged upstream, so the lock is authoritative-intent, not yet live on the branch; (b) the server doesn't yet persist the bundle bytes (FFX_BLOB byte-write is a TODO). These are external blockers on end-to-end success, not on the CLI's correctness. `--use-salesforce-pages` still has no corresponding server-side field (PR #118209 did NOT add one), so the `pages` form field has no server-side home — that's a separate open item (spec §2.4). Exactly one call per invocation either way, no retry/poll loop, no `--wait` flag (REQ-101, REQ-303). | spec §2.3 AC1 (REQ-101), §2.5, §7 (REQ-303) | | 2.6 | Map response → `UiBundleUploadResult`. On `Queued` (the only response shape the merged Core contract actually documents — the `202 Accepted` representation is fixed at `{ "jobId": "", "status": "Queued" }`, spec §2.5): print human success block (spec §2.6) or return object for `--json` (REQ-106, REQ-107). On synchronous `Failed` — **defensive handling for a response shape not expected in practice**, since `POST` is async-only and the merged contract never returns a job-shaped `Failed` body from the `POST` itself (spec §2.6 failure block marked defensive, §3.1 case 5, §3.2; see §1 above): print human failure block to **stderr** (REQ-108) or return object with `message` verbatim for `--json` (REQ-109) — this is a normal returned result, not a thrown error (server responded successfully, it just rejected the bundle). The `Failed`-status human block — the "Upload failed" line and its `Job ID:`/`Message:` labels — **must resolve via `messages.getMessage()` from the keys added in Step 1.2**, NOT inlined as string literals (spec §6.3). Implement the branch for completeness and AC2 coverage, not because it's a live path today. Note the §2.6 human success block is path-dependent: with `--bundle-dir` it prints a `Packaging bundle source... done` step (the real SDR pass), with `--zip-file` that step is a trivial no-op — both then print the identical `Staging and initiating upload... done` step. Falsifiable: `grep 'Upload failed' src/commands/ui-bundle/upload.ts` → zero matches, `grep 'Job ID:' src/commands/ui-bundle/upload.ts` → zero matches (the moved literals are gone — the `Failed` block lives in the message file), and the `Failed`-status case in the unit test (Step 4.1) confirms the output still renders correctly from the message file. | spec §2.3 AC2 (REQ-106–109), §6.3 | -| 2.7 | Wire CLI-side error handling via `messages.createError()`: throw `messages.createError('error.uiBundleUploadAuthError', [message])` on connection/auth failure, `messages.createError('error.uiBundleUploadNetworkError', [message])` on no-HTTP-response network failure, `messages.createError('error.uiBundleUploadValidationError', [message])` on server-side rejection (an HTTP error with an `errorCode`). The derived error `name`s (`UiBundleUploadAuthError`, `UiBundleUploadNetworkError`, `UiBundleUploadValidationError`) are UNCHANGED — the mechanism shifts from inline `new SfError(message, name)` to `messages.createError(key, [message])`, but the observable error names stay the same. The message keys were already added in Step 1.2: `# error.uiBundleUploadAuthError`, `# error.uiBundleUploadNetworkError`, `# error.uiBundleUploadValidationError` — each wraps the server/framework text as a single `%s` token. The `SfError` import is no longer needed (it's unreferenced after this change). | spec §2.3 AC3 (REQ-110) | +| 2.7 | Wire CLI-side error handling via `messages.createError()`: throw `messages.createError('error.uiBundleUploadAuthError', [message])` on connection/auth failure, `messages.createError('error.uiBundleUploadNetworkError', [message])` on no-HTTP-response network failure, `messages.createError('error.uiBundleUploadError', [message])` on server-side rejection (an HTTP error with an `errorCode`). The derived error `name`s (`UiBundleUploadAuthError`, `UiBundleUploadNetworkError`, `UiBundleUploadError`) are UNCHANGED — the mechanism shifts from inline `new SfError(message, name)` to `messages.createError(key, [message])`, but the observable error names stay the same. The message keys were already added in Step 1.2: `# error.uiBundleUploadAuthError`, `# error.uiBundleUploadNetworkError`, `# error.uiBundleUploadError` — each wraps the server/framework text as a single `%s` token. The `SfError` import is no longer needed (it's unreferenced after this change). | spec §2.3 AC3 (REQ-110) | | 2.8 | Surface server error messages verbatim — no truncation/re-interpretation, whether from an HTTP error body or a `Failed`-status `message` field. **Server/framework-supplied messages are pass-through** (spec §6.3 carve-out (a)): a caught `error.message` from the org connection, an HTTP error body, or any externally-sourced error text is relayed as-is — it is NOT a hardcoded literal and is out of scope for the §6.3 rule. | spec §2.3 AC3 (REQ-111), §6.3 | **Comment style for `upload.ts`:** every code comment written in Phase 2 (2.1–2.8) follows spec §6.1 Code Comment Guidelines: diff --git a/.sdd/ui-bundle-upload/spec.md b/.sdd/ui-bundle-upload/spec.md index 7c980c6..d2753d3 100644 --- a/.sdd/ui-bundle-upload/spec.md +++ b/.sdd/ui-bundle-upload/spec.md @@ -510,7 +510,7 @@ All customer-facing output messages — whether success text, info lines, or thr 1. **All customer-facing output messages are defined in `messages/ui-bundle.upload.md` and referenced via `messages.getMessage()`.** Never inline customer-facing strings as string literals in the command source. This extends the oclif/sf-plugins-core convention for summaries/descriptions/examples to all runtime output the user sees. 2. **Server/framework-supplied messages surfaced verbatim are pass-through, not authored strings.** A caught `error.message` from the org connection, an HTTP error body, or any other externally-sourced error text is relayed as-is (§2.3 AC3 REQ-111 requires verbatim surfacing) — it is **not** a hardcoded literal and is out of scope for this rule. -3. **Thrown errors are constructed via `messages.createError(key, tokens)`, and the stable machine-readable error name is derived automatically from the message key.** Per `@salesforce/core`'s `Messages.createError()` convention: strip the `error.` prefix from the key, uppercase the first letter, and the remainder of the key must end in a properly-cased literal `Error` — e.g. key `error.uiBundleUploadValidationError` derives the `name` `UiBundleUploadValidationError`. The error name is not passed as a separate literal; it's computed from the key itself, so the message file and the thrown error's name stay consistent. +3. **Thrown errors are constructed via `messages.createError(key, tokens)`, and the stable machine-readable error name is derived automatically from the message key.** Per `@salesforce/core`'s `Messages.createError()` convention: strip the `error.` prefix from the key, uppercase the first letter, and the remainder of the key must end in a properly-cased literal `Error` — e.g. key `error.uiBundleUploadError` derives the `name` `UiBundleUploadError`. The error name is not passed as a separate literal; it's computed from the key itself, so the message file and the thrown error's name stay consistent. The command currently inlines three customer-facing strings that this rule requires moving into `messages/ui-bundle.upload.md`: the empty-bundle-dir `SfError` message (`'The bundle source directory is empty.'` in `compressDirectory`), the compression-failure `SfError` message (`'Failed to compress the bundle source directory.'`), and the `Failed`-status human block (`'Upload failed'` and its `Job ID:` / `Message:` labels) — this is the "Upload failed" text the user specifically called out as residing separately in upload.ts. Note: `messages/ui-bundle.upload.md` already defines unused `# error.*` keys (`error.upload-failed`, `error.auth-failed`, `error.network-failed`, `error.validation-failed`) that the code does not currently reference — the guideline's intent is that authored output routes through such message keys rather than duplicating strings inline. diff --git a/messages/ui-bundle.upload.md b/messages/ui-bundle.upload.md index b54e6a1..29648d5 100644 --- a/messages/ui-bundle.upload.md +++ b/messages/ui-bundle.upload.md @@ -72,6 +72,6 @@ Authentication error: %s Network error: %s -# error.uiBundleUploadValidationError +# error.uiBundleUploadError -Validation error: %s +Error: %s diff --git a/src/commands/ui-bundle/upload.ts b/src/commands/ui-bundle/upload.ts index e62bb81..1b59346 100644 --- a/src/commands/ui-bundle/upload.ts +++ b/src/commands/ui-bundle/upload.ts @@ -43,19 +43,27 @@ function collectFiles(root: string): string[] { return files; } -/** Compress a source directory into a zip Buffer using jszip. */ +/** + * Compress a source directory into a zip Buffer using jszip. + * + * Every entry is nested under a single top-level wrapper directory (the source directory's + * basename), since the Connect API's ui-bundle deploy endpoint rejects zips whose entries live + * at the zip root — it requires all entries to share one common top-level directory, but doesn't + * care what that directory is named. + */ async function compressDirectory(dir: string): Promise { const zip = new JSZip(); + const wrapperDir = basename(dir); let fileCount = 0; for (const file of collectFiles(dir)) { // Entry paths inside a zip are always posix; normalize Windows separators. - const entryPath = relative(dir, file).split(sep).join('/'); + const entryPath = `${wrapperDir}/${relative(dir, file).split(sep).join('/')}`; zip.file(entryPath, readFileSync(file)); fileCount++; } // An empty directory produces no zip entries; reject rather than POST an empty bundle. if (fileCount === 0) { - throw messages.createError('error.uiBundleUploadValidationError', [messages.getMessage('error.bundle-dir-empty')]); + throw messages.createError('error.uiBundleUploadError', [messages.getMessage('error.bundle-dir-empty')]); } // JSZip's generateAsync resolves with a Buffer or rejects; no silent-failure path exists. return zip.generateAsync({ type: 'nodebuffer', compression: 'DEFLATE', compressionOptions: { level: 3 } }); @@ -153,7 +161,7 @@ export default class UiBundleUpload extends SfCommand { throw messages.createError('error.uiBundleUploadAuthError', [errorMessage]); } if (errorCode) { - throw messages.createError('error.uiBundleUploadValidationError', [errorMessage]); + throw messages.createError('error.uiBundleUploadError', [errorMessage]); } throw messages.createError('error.uiBundleUploadNetworkError', [errorMessage]); } diff --git a/test/commands/ui-bundle/upload.test.ts b/test/commands/ui-bundle/upload.test.ts index afa7574..2006c3f 100644 --- a/test/commands/ui-bundle/upload.test.ts +++ b/test/commands/ui-bundle/upload.test.ts @@ -489,11 +489,37 @@ describe('ui-bundle:upload command unit tests', () => { expect(sent.length).to.be.greaterThan(100); }); + it('--bundle-dir -> every zip entry is nested under a / wrapper directory', async () => { + const requestStub = $$.SANDBOX.stub().resolves({ jobId: '0BXxx0000000014', status: 'Queued' }); + $$.fakeConnectionRequest = requestStub; + stubSfCommandUx($$.SANDBOX); + const bundleDir = createBundleDirFixture(); + const wrapperDir = basename(bundleDir); + + await UiBundleUpload.run( + ['--bundle-dir', bundleDir, '--use-salesforce-pages', '--target-org', testOrg.username], + import.meta.url + ); + + expect(requestStub.calledOnce).to.be.true; + const sent = bundleBufferFromRequest(requestStub.firstCall.args[0]); + const zip = await JSZip.loadAsync(sent); + const entries = Object.keys(zip.files); + + // The Connect API rejects zips whose entries live at the root; every entry must be nested + // under a single common top-level directory. We use the bundle dir's basename for it. + expect(entries).to.include(`${wrapperDir}/index.html`); + expect(entries).to.include(`${wrapperDir}/src/app.js`); + // No entry lives at the zip root. + expect(entries.every((e) => e.startsWith(`${wrapperDir}/`))).to.be.true; + }); + it('--bundle-dir with dotfiles -> dotfiles and dot-directories are filtered out', async () => { const requestStub = $$.SANDBOX.stub().resolves({ jobId: '0BXxx0000000005', status: 'Queued' }); $$.fakeConnectionRequest = requestStub; stubSfCommandUx($$.SANDBOX); const bundleDir = createBundleDirWithDotfilesFixture(); + const wrapperDir = basename(bundleDir); await UiBundleUpload.run( ['--bundle-dir', bundleDir, '--use-salesforce-pages', '--target-org', testOrg.username], @@ -505,9 +531,9 @@ describe('ui-bundle:upload command unit tests', () => { const zip = await JSZip.loadAsync(sent); const entries = Object.keys(zip.files); - // Assert non-dotfiles are present. - expect(entries).to.include('index.html'); - expect(entries).to.include('src/app.js'); + // Assert non-dotfiles are present, nested under the wrapper directory. + expect(entries).to.include(`${wrapperDir}/index.html`); + expect(entries).to.include(`${wrapperDir}/src/app.js`); // Assert dotfiles and dot-directory contents are absent. expect(entries.some((e) => e.includes('.env'))).to.be.false; @@ -521,6 +547,7 @@ describe('ui-bundle:upload command unit tests', () => { this.skip(); return; } + const wrapperDir = basename(bundleDir); const requestStub = $$.SANDBOX.stub().resolves({ jobId: '0BXxx0000000006', status: 'Queued' }); $$.fakeConnectionRequest = requestStub; @@ -536,18 +563,18 @@ describe('ui-bundle:upload command unit tests', () => { const zip = await JSZip.loadAsync(sent); const entries = Object.keys(zip.files); - // Non-symlink entries are still present. - expect(entries).to.include('index.html'); - expect(entries).to.include('src/app.js'); + // Non-symlink entries are still present, nested under the wrapper directory. + expect(entries).to.include(`${wrapperDir}/index.html`); + expect(entries).to.include(`${wrapperDir}/src/app.js`); // The symlinked file and the symlinked directory's nested file are both bundled. - expect(entries).to.include('linked.js'); - expect(entries).to.include('linked-dir/nested.js'); + expect(entries).to.include(`${wrapperDir}/linked.js`); + expect(entries).to.include(`${wrapperDir}/linked-dir/nested.js`); - const linkedFileContent = await zip.files['linked.js'].async('string'); + const linkedFileContent = await zip.files[`${wrapperDir}/linked.js`].async('string'); expect(linkedFileContent).to.equal('console.log("linked file");'); - const linkedDirFileContent = await zip.files['linked-dir/nested.js'].async('string'); + const linkedDirFileContent = await zip.files[`${wrapperDir}/linked-dir/nested.js`].async('string'); expect(linkedDirFileContent).to.equal('console.log("linked dir");'); }); From f6736c3cba0f3b4bb1de3e98950ad171b87202e9 Mon Sep 17 00:00:00 2001 From: william-xie Date: Thu, 16 Jul 2026 13:21:05 -0700 Subject: [PATCH 2/3] fix: additional error references --- .sdd/ui-bundle-upload/plan.md | 2 +- .sdd/ui-bundle-upload/spec.md | 6 +++--- test/commands/ui-bundle/upload.test.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.sdd/ui-bundle-upload/plan.md b/.sdd/ui-bundle-upload/plan.md index 3ecf1f8..07fb85c 100644 --- a/.sdd/ui-bundle-upload/plan.md +++ b/.sdd/ui-bundle-upload/plan.md @@ -84,7 +84,7 @@ None of the remaining non-goals (301/303/304/305) require a dedicated implementa - One short line, not a paragraph. - No requirement/AC/spec-ID citations (no `REQ-112`, `AC2`, `§2.5`, etc.) — state the reasoning directly instead. -**Output message style for `upload.ts`:** all customer-facing output messages — success text, info lines, or thrown `SfError` message strings — are defined in `messages/ui-bundle.upload.md` and referenced via `messages.getMessage()` (spec §6.3), never inlined as string literals. Two carve-outs: (a) server/framework messages surfaced verbatim (a caught `error.message`) are pass-through, not authored strings (Step 2.8); (b) the `SfError` name/error-code second argument (e.g. `'UiBundleUploadValidationError'`) stays inline as a machine identifier, not customer-facing prose. +**Output message style for `upload.ts`:** all customer-facing output messages — success text, info lines, or thrown `SfError` message strings — are defined in `messages/ui-bundle.upload.md` and referenced via `messages.getMessage()` (spec §6.3), never inlined as string literals. Two carve-outs: (a) server/framework messages surfaced verbatim (a caught `error.message`) are pass-through, not authored strings (Step 2.8); (b) the `SfError` name/error-code second argument (e.g. `'UiBundleUploadError'`) stays inline as a machine identifier, not customer-facing prose. **Non-regression checkpoint 1** (see §6 below) — run here, after Step 2.8, before touching any packaging file. diff --git a/.sdd/ui-bundle-upload/spec.md b/.sdd/ui-bundle-upload/spec.md index d2753d3..1dacf7c 100644 --- a/.sdd/ui-bundle-upload/spec.md +++ b/.sdd/ui-bundle-upload/spec.md @@ -72,7 +72,7 @@ **AC3 (REQ-110–112) — Error semantics** -- [ ] **110.** The _actual_ synchronous-failure path: an HTTP-level 4xx/5xx response from the `POST` call itself (no job id, no valid job-shaped body — e.g. the server's own early size/content-type rejection per §2.5, or auth failure, or no HTTP response at all) → thrown `SfError` with a distinct CLI-side name (`UiBundleUploadAuthError`/`UiBundleUploadNetworkError`/`UiBundleUploadValidationError`), separate from a server `Failed` status result object (108/109). Caveat: per §2.5's Known Limitations, the size/content-type rejection sub-case is not yet live — it's a defensive/forward-looking path, not one that can be exercised against the current endpoint. The auth-failure and no-HTTP-response causes in this same path remain valid today. +- [ ] **110.** The _actual_ synchronous-failure path: an HTTP-level 4xx/5xx response from the `POST` call itself (no job id, no valid job-shaped body — e.g. the server's own early size/content-type rejection per §2.5, or auth failure, or no HTTP response at all) → thrown `SfError` with a distinct CLI-side name (`UiBundleUploadAuthError`/`UiBundleUploadNetworkError`/`UiBundleUploadError`), separate from a server `Failed` status result object (108/109). Caveat: per §2.5's Known Limitations, the size/content-type rejection sub-case is not yet live — it's a defensive/forward-looking path, not one that can be exercised against the current endpoint. The auth-failure and no-HTTP-response causes in this same path remain valid today. - [ ] **111.** Server error message — whether from an HTTP error body (110) or, defensively, a `Failed.message` (108/109) — surfaced verbatim, no rewriting or truncation. - [ ] **112.** No client-side zip-content validation, ever. @@ -376,7 +376,7 @@ Status values (`Queued`/`InProgress`/`Succeeded`/`Failed`) match the server-side 1. **HTTP 4xx/5xx server rejection from the `POST` itself (size/content-type/validation)** - **When:** the server synchronously rejects the request — e.g. its early size/content-type check (§2.5) — returning an HTTP error with no job id and no job-shaped body. Caveat: per §2.5's Known Limitations, this size/content-type sub-case is not yet live against the current endpoint — it's a defensive/forward-looking path, kept here for when server-side validation lands. - - **Display:** thrown `UiBundleUploadValidationError` (`SfError` from `@salesforce/core`), server message surfaced verbatim (REQ-111), no rewriting or truncation. + - **Display:** thrown `UiBundleUploadError` (`SfError` from `@salesforce/core`), server message surfaced verbatim (REQ-111), no rewriting or truncation. - **Action:** exit 1; no result object emitted. This is the _actual_ synchronous-failure path (REQ-110), distinct from the defensive `Failed` result object (§3.1 case 5 / AC2 108–109). 2. **Auth failure** @@ -445,7 +445,7 @@ Status values (`Queued`/`InProgress`/`Succeeded`/`Failed`) match the server-side - [ ] `--api-version` omitted (defaulted) → resolved value passed into `getConnection()`, and the connection's resolved `getApiVersion()` is still checked against the floor unconditionally (AC10 117c/117d). - [ ] `Queued` response → human success block and `--json` shape (§2.6). - [ ] `Failed` response (defensive) → human failure block and `--json` shape (§2.6). -- [ ] Each CLI-side `SfError` name asserted: `UiBundleUploadValidationError` / `UiBundleUploadNetworkError` / `UiBundleUploadAuthError` / `UiBundleUploadApiVersionError`. +- [ ] Each CLI-side `SfError` name asserted: `UiBundleUploadError` / `UiBundleUploadNetworkError` / `UiBundleUploadAuthError` / `UiBundleUploadApiVersionError`. - [ ] Preview-state warning emitted (`state = 'preview'`) — not suppressed under `--json`'s result payload. - [ ] No customer-facing output literal is inlined in `upload.ts` — all such output resolves via `messages.getMessage()` per §6.3. - [ ] Lint, build, and license-header checks clean on all new `.ts` files. diff --git a/test/commands/ui-bundle/upload.test.ts b/test/commands/ui-bundle/upload.test.ts index 2006c3f..58f6993 100644 --- a/test/commands/ui-bundle/upload.test.ts +++ b/test/commands/ui-bundle/upload.test.ts @@ -618,7 +618,7 @@ describe('ui-bundle:upload command unit tests', () => { } }); - it('HTTP error with errorCode -> throws UiBundleUploadValidationError, message verbatim', async () => { + it('HTTP error with errorCode -> throws UiBundleUploadError, message verbatim', async () => { const serverError = new Error('The org rejected the bundle: unsupported file type') as Error & { errorCode: string; }; @@ -634,7 +634,7 @@ describe('ui-bundle:upload command unit tests', () => { expect.fail('should have thrown'); } catch (e) { const err = e as Error & { name: string; message: string }; - expect(err.name).to.equal('UiBundleUploadValidationError'); + expect(err.name).to.equal('UiBundleUploadError'); expect(err.message).to.include('The org rejected the bundle: unsupported file type'); } }); From 0373c662b18d051cb55242d05fcaee576554cfbc Mon Sep 17 00:00:00 2001 From: william-xie Date: Fri, 17 Jul 2026 14:32:00 -0700 Subject: [PATCH 3/3] fix: add basic file name validation at CLI level --- messages/ui-bundle.upload.md | 6 +- src/commands/ui-bundle/upload.ts | 35 +++++---- test/commands/ui-bundle/upload.test.ts | 99 +++++++++++++++++++++++++- 3 files changed, 123 insertions(+), 17 deletions(-) diff --git a/messages/ui-bundle.upload.md b/messages/ui-bundle.upload.md index 29648d5..7c4d650 100644 --- a/messages/ui-bundle.upload.md +++ b/messages/ui-bundle.upload.md @@ -26,7 +26,7 @@ Name to associate with the uploaded UI Bundle. # flags.bundle-name.description -A human-readable name for the UI Bundle. If not specified, defaults to the base name of --bundle-dir or --zip-file, with any .zip extension removed. +A human-readable name for the UI Bundle. If not specified, defaults to the base name of --bundle-dir or --zip-file, with any .zip extension removed. When used with --bundle-dir, this name also becomes the zip's top-level directory name, so it must start with a letter and contain only letters, numbers, and underscores after that. # examples @@ -60,6 +60,10 @@ Upload failed The bundle source directory is empty. +# error.bundle-dir-name-invalid + +The bundle name '%s' isn't valid: it must start with a letter, and contain only letters, numbers, and underscores after that. This name is used both as the requested bundle name and as the zip's top-level directory name, and your org rejects any other characters there. Pass a valid name explicitly with --bundle-name, or rename the --bundle-dir directory. + # error.uiBundleUploadApiVersionError API version %s isn't supported by this command; --api-version must be %s or later. diff --git a/src/commands/ui-bundle/upload.ts b/src/commands/ui-bundle/upload.ts index 407929a..1c9e091 100644 --- a/src/commands/ui-bundle/upload.ts +++ b/src/commands/ui-bundle/upload.ts @@ -28,6 +28,9 @@ const messages = Messages.loadMessages('@salesforce/plugin-ui-bundle-dev', 'ui-b // Versions below this floor aren't supported by the UI Bundle deploy endpoint. const MINIMUM_SUPPORTED_API_VERSION = 67; +// Bundle names must start with a letter and can contain other letters, digits, or underscores +const BUNDLE_DIR_NAME_PATTERN = /^[A-Za-z][A-Za-z0-9_]*$/; + /** Recursively collect absolute paths of every file under a directory. */ function collectFiles(root: string): string[] { const files: string[] = []; @@ -46,14 +49,14 @@ function collectFiles(root: string): string[] { /** * Compress a source directory into a zip Buffer using jszip. * - * Every entry is nested under a single top-level wrapper directory (the source directory's - * basename), since the Connect API's ui-bundle deploy endpoint rejects zips whose entries live - * at the zip root — it requires all entries to share one common top-level directory, but doesn't - * care what that directory is named. + * Every entry is nested under a single top-level wrapper directory, since the Connect API's + * ui-bundle deploy endpoint rejects zips whose entries live at the zip root — it requires all + * entries to share one common top-level directory. The caller must supply a `wrapperDir` that + * already satisfies the server's directory-name allowlist (UiBundleDeployService.validateZip); + * this function doesn't validate it. */ -async function compressDirectory(dir: string): Promise { +async function compressDirectory(dir: string, wrapperDir: string): Promise { const zip = new JSZip(); - const wrapperDir = basename(dir); let fileCount = 0; for (const file of collectFiles(dir)) { // Entry paths inside a zip are always posix; normalize Windows separators. @@ -122,20 +125,26 @@ export default class UiBundleUpload extends SfCommand { const bundleDir = flags['bundle-dir']; let zipBuffer: Buffer; let zipFilename: string; + let bundleName: string; if (bundleDir) { - zipBuffer = await compressDirectory(bundleDir); - zipFilename = `${basename(bundleDir)}.zip`; + bundleName = flags['bundle-name'] ?? basename(bundleDir); + if (!BUNDLE_DIR_NAME_PATTERN.test(bundleName)) { + throw messages.createError('error.uiBundleUploadError', [ + messages.getMessage('error.bundle-dir-name-invalid', [bundleName]), + ]); + } + zipBuffer = await compressDirectory(bundleDir, bundleName); + zipFilename = `${bundleName}.zip`; } else { const zipFile = flags['zip-file']!; zipBuffer = readFileSync(zipFile); zipFilename = basename(zipFile); + // Defaults to the zip's base name (extension stripped) when --bundle-name is omitted. + // Falls back to the unstripped filename if stripping would leave an empty string (e.g. a file literally named ".zip"). + const strippedZipFilename = zipFilename.replace(/\.zip$/i, ''); + bundleName = flags['bundle-name'] ?? (strippedZipFilename || zipFilename); } - // Defaults to the bundle source's base name (zip extension stripped) when --bundle-name is omitted. - // Falls back to the unstripped filename if stripping would leave an empty string (e.g. a file literally named ".zip"). - const strippedZipFilename = zipFilename.replace(/\.zip$/i, ''); - const bundleName = flags['bundle-name'] ?? (strippedZipFilename || zipFilename); - // Step 3: Build the multipart body and issue a single synchronous POST, no retry/poll loop. // We send form.getBuffer() (the fully-assembled multipart Buffer) since jsforce's instanceof FormData check fails across differing form-data module copies. const form = new FormData(); diff --git a/test/commands/ui-bundle/upload.test.ts b/test/commands/ui-bundle/upload.test.ts index d45b7b7..a194324 100644 --- a/test/commands/ui-bundle/upload.test.ts +++ b/test/commands/ui-bundle/upload.test.ts @@ -41,21 +41,37 @@ function createZipFixture(): string { /** * Materialize an uncompressed source directory for the `--bundle-dir` path. * A couple of nested files are enough to exercise SDR's recursive compression. + * + * The mkdtemp prefix is deliberately hyphen-free: `compressDirectory` (by default, absent + * --bundle-name) uses the directory's basename as the zip's top-level wrapper directory, which + * must satisfy the server's allowlist ([A-Za-z][A-Za-z0-9_]*). Node's mkdtemp suffix is always + * alphanumeric, so a hyphen-free prefix keeps the basename allowlist-valid for tests that aren't + * specifically exercising the naming guard. */ function createBundleDirFixture(): string { - const dir = mkdtempSync(join(tmpdir(), 'upload-test-dir-')); + const dir = mkdtempSync(join(tmpdir(), 'uploadtestdir')); mkdirSync(join(dir, 'src')); writeFileSync(join(dir, 'index.html'), ''); writeFileSync(join(dir, 'src', 'app.js'), 'console.log("hi");'); return dir; } +/** + * Materialize an uncompressed source directory whose basename fails the server's directory-name + * allowlist (contains hyphens), to exercise the --bundle-dir naming guard. + */ +function createHyphenatedBundleDirFixture(): string { + const dir = mkdtempSync(join(tmpdir(), 'upload-test-dir-')); + writeFileSync(join(dir, 'index.html'), ''); + return dir; +} + /** * Materialize an uncompressed source directory with dotfiles and dot-directories. * Used to verify the dotfile filter path. */ function createBundleDirWithDotfilesFixture(): string { - const dir = mkdtempSync(join(tmpdir(), 'upload-test-dir-dotfiles-')); + const dir = mkdtempSync(join(tmpdir(), 'uploadtestdirdotfiles')); mkdirSync(join(dir, 'src')); mkdirSync(join(dir, '.git')); writeFileSync(join(dir, 'index.html'), ''); @@ -75,7 +91,7 @@ function createBundleDirWithDotfilesFixture(): string { * unprivileged Windows CI without Developer Mode), so callers can skip the test. */ function createBundleDirWithSymlinksFixture(): string | undefined { - const dir = mkdtempSync(join(tmpdir(), 'upload-test-dir-symlinks-')); + const dir = mkdtempSync(join(tmpdir(), 'uploadtestdirsymlinks')); mkdirSync(join(dir, 'src')); writeFileSync(join(dir, 'index.html'), ''); writeFileSync(join(dir, 'src', 'app.js'), 'console.log("hi");'); @@ -369,6 +385,83 @@ describe('ui-bundle:upload command unit tests', () => { expect(sent).to.include(`Content-Type: application/json\r\n\r\n{"requestedName":"${expectedName}"}`); }); + it('--bundle-dir with a hyphenated basename and no --bundle-name -> throws UiBundleUploadError, no network call', async () => { + const requestStub = $$.SANDBOX.stub().resolves({ jobId: '0BXxx0000000015', status: 'Queued' }); + $$.fakeConnectionRequest = requestStub; + stubSfCommandUx($$.SANDBOX); + const bundleDir = createHyphenatedBundleDirFixture(); + + try { + await UiBundleUpload.run( + ['--bundle-dir', bundleDir, '--use-salesforce-pages', '--target-org', testOrg.username], + import.meta.url + ); + expect.fail('should have thrown'); + } catch (e) { + const err = e as Error & { name: string; message: string }; + expect(err.name).to.equal('UiBundleUploadError'); + expect(err.message).to.include(basename(bundleDir)); + } + expect(requestStub.called).to.be.false; + }); + + it('--bundle-dir with a hyphenated basename and a valid --bundle-name -> reconciles the wrapper dir with --bundle-name', async () => { + const requestStub = $$.SANDBOX.stub().resolves({ jobId: '0BXxx0000000016', status: 'Queued' }); + $$.fakeConnectionRequest = requestStub; + stubSfCommandUx($$.SANDBOX); + const bundleDir = createHyphenatedBundleDirFixture(); + + await UiBundleUpload.run( + [ + '--bundle-dir', + bundleDir, + '--use-salesforce-pages', + '--target-org', + testOrg.username, + '--bundle-name', + 'my_valid_bundle', + ], + import.meta.url + ); + + expect(requestStub.calledOnce).to.be.true; + const sent = bundleBufferFromRequest(requestStub.firstCall.args[0]); + const sentText = sent.toString('utf8'); + expect(sentText).to.include('Content-Type: application/json\r\n\r\n{"requestedName":"my_valid_bundle"}'); + // The zip's top-level wrapper directory matches --bundle-name, not the hyphenated dir basename. + const zip = await JSZip.loadAsync(sent); + const entries = Object.keys(zip.files); + expect(entries).to.include('my_valid_bundle/index.html'); + }); + + it('--bundle-dir with an invalid --bundle-name (leading digit) -> throws UiBundleUploadError, no network call', async () => { + const requestStub = $$.SANDBOX.stub().resolves({ jobId: '0BXxx0000000017', status: 'Queued' }); + $$.fakeConnectionRequest = requestStub; + stubSfCommandUx($$.SANDBOX); + const bundleDir = createBundleDirFixture(); + + try { + await UiBundleUpload.run( + [ + '--bundle-dir', + bundleDir, + '--use-salesforce-pages', + '--target-org', + testOrg.username, + '--bundle-name', + '2048-app', + ], + import.meta.url + ); + expect.fail('should have thrown'); + } catch (e) { + const err = e as Error & { name: string; message: string }; + expect(err.name).to.equal('UiBundleUploadError'); + expect(err.message).to.include('2048-app'); + } + expect(requestStub.called).to.be.false; + }); + it('--zip-file named ".zip" -> requestedName falls back to the unstripped filename, not empty', async () => { const requestStub = $$.SANDBOX.stub().resolves({ jobId: '0BXxx0000000011', status: 'Queued' }); $$.fakeConnectionRequest = requestStub;