Validate Edge extension product config by rissrice2105-agent · Pull Request #631 · profullstack/sh1pt

rissrice2105-agent · 2026-06-05T22:49:17Z

Fixes #630.

Changes:

require browser-edge productId before package or publish work starts
reject product IDs that are not single URL path segments
trim and validate optional sourceDir and notes text
use the normalized productId consistently in plans, logs, API URLs, and publish result IDs
add regression tests proving invalid config fails before zip/API work

Validation:

vitest run packages/targets/browser-edge/src/index.test.ts
tsc -p packages/targets/browser-edge/tsconfig.json --noEmit

greptile-apps · 2026-06-05T22:52:41Z

Greptile Summary

This PR adds upfront config validation to the browser-edge target: a required productId, optional-but-non-blank sourceDir and notes, and a character-exclusion regex to reject values that contain URL meta-characters. The validated, trimmed productId is then used consistently across plans, logs, API URLs, and result IDs.

requireProductId blocks slash, backslash, ?, #, and control characters, but the regex does not cover . or .. — WHATWG URL normalisation resolves products/../submissions/… before the request is sent, silently routing both API calls to wrong endpoints.
Two new regression tests cover blank and slash-containing IDs, as well as blank sourceDir and notes, but there is no test for the .. case that slips through the regex.
requireProductId is invoked three times per build call (directly plus inside packageArtifact and packagePlan); threading the pre-validated value would remove the redundancy.

Confidence Score: 3/5

The change is mostly safe, but the requireProductId regex has a gap that allows '..' as a product ID, causing both Edge API calls to be routed to wrong endpoints via URL normalisation — the exact kind of misconfiguration this PR is trying to prevent.

The validation logic correctly rejects blank values and obvious path separators, but '..' passes the regex and silently redirects the upload and submit network calls to incorrect API paths. The gap is in the core validation function introduced by this PR and affects every call to build and ship.

The requireProductId function in packages/targets/browser-edge/src/index.ts needs a dot-segment guard, and the test file should add a '..' case to the rejection test.

Important Files Changed

Filename	Overview
packages/targets/browser-edge/src/index.ts	Adds requireText, optionalText, and requireProductId validation helpers; uses them consistently in build, ship, and plan helpers. The character-exclusion regex in requireProductId misses the '.' / '..' dot-segment cases, which WHATWG URL normalisation resolves into path traversal at the API call sites.
packages/targets/browser-edge/src/index.test.ts	Replaces the old path-separator test with two new validation-rejection tests covering blank productId, slash-containing productId, blank sourceDir, and blank notes. No test for '..' as productId, which is the case the regex misses.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build / ship called] --> B[requireProductId]
    B -->|blank / whitespace| E1[throw: browser-edge requires productId]
    B -->|contains \\ / ? # or control char| E2[throw: productId must be a single URL path segment]
    B -->|'.' or '..'| PASS[passes through ⚠️]
    B -->|valid| C{dryRun?}
    PASS --> C
    C -->|yes| D[write edge-package.json plan]
    C -->|no - build| F[validate manifest.json → zip → return artifact]
    C -->|no - ship| G[OAuth token → PUT upload URL → POST submit URL]
    G -->|productId = '..'| H[URL normalises to /v1/submissions/… ⚠️]
    D --> Z[return artifact path]
    F --> Z
    G --> I[return id / url / meta]

_{Reviews (1): Last reviewed commit: "Validate Edge extension product config" | Re-trigger Greptile}

greptile-apps · 2026-06-05T22:52:45Z

+function requireProductId(value: string | undefined): string {
+  const productId = requireText(value, 'productId');
+  if (/[\\/?#\x00-\x1F\x7F]/.test(productId)) {
+    throw new Error('browser-edge productId must be a single URL path segment');
+  }
+  return productId;
+}


.. bypasses the path-segment guard and traverses the API URL

The regex /[\\/?#\x00-\x1F\x7F]/ does not block . or ... Because fetch parses its URL argument via the WHATWG URL algorithm, a productId of .. causes the upload and submit URLs to be silently normalised: …/v1/products/../submissions/… → …/v1/submissions/…. Both API calls then hit the wrong endpoint and the access token is included in those misdirected requests. Add a dot-segment check after requireText returns, e.g. if (productId === '.' || productId === '..') throw new Error('browser-edge productId must be a single URL path segment');, or switch to an allowlist regex.

greptile-apps · 2026-06-05T22:52:45Z

+function optionalText(value: string | undefined, name: string): string | undefined {
+  if (value === undefined) return undefined;
+  const trimmed = requireText(value, name);
+  return trimmed;
+}


The optionalText helper stores the requireText return value in a local trimmed variable only to immediately return it. Returning the result directly is equivalent and removes the extra variable.

Suggested change

function optionalText(value: string | undefined, name: string): string | undefined {

if (value === undefined) return undefined;

const trimmed = requireText(value, name);

return trimmed;

}

function optionalText(value: string | undefined, name: string): string | undefined {

if (value === undefined) return undefined;

return requireText(value, name);

}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

greptile-apps · 2026-06-05T22:52:46Z

  async build(ctx, config) {
+    const productId = requireProductId(config.productId);
    const src = sourceDir(ctx, config);
    const zipPath = packageArtifact(ctx, config);


requireProductId called three times per build

build() validates productId on line 68, then packageArtifact validates it again (line 45), and packagePlan validates it a third time (line 49). Threading the already-validated productId into those helpers as a parameter would remove the redundancy.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

github-actions · 2026-06-06T07:09:51Z