Merge pull request #148 from Lykhoyda/fix/134.5-workflow-and-correctness

Lykhoyda · web-flow · commit 2f7e64a3e300 · 2026-05-12T21:30:33.000+02:00
fix: workflow + correctness sweep (Phase 134.5)
diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json
@@ -9,7 +9,7 @@
     {
       "name": "rn-dev-agent",
       "description": "AI agent that fully tests React Native features on simulator/emulator — navigates the app, verifies UI, walks user flows, and confirms internal state.",
-      "version": "0.44.34",
+      "version": "0.44.35",
       "source": "./",
       "category": "mobile-development",
       "homepage": "https://github.com/Lykhoyda/rn-dev-agent"
diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json
@@ -1,6 +1,6 @@
 {
   "name": "rn-dev-agent",
-  "version": "0.44.34",
+  "version": "0.44.35",
   "description": "AI agent that fully tests React Native features on simulator/emulator — navigates the app, verifies UI, walks user flows, and confirms internal state.",
   "author": {
     "name": "Anton Lykhoyda",
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -15,9 +15,9 @@ jobs:
         working-directory: scripts/cdp-bridge
 
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2 — Phase 134.5 SHA pin
 
-      - uses: actions/setup-node@v4
+      - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e  # v6.4.0 — Phase 134.5 SHA pin
         with:
           node-version: 22
           cache: npm
@@ -38,5 +38,5 @@ jobs:
     name: Version sync check
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2 — Phase 134.5 SHA pin
       - run: bash scripts/sync-versions.sh
diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml
@@ -12,10 +12,12 @@ on:
       - 'CHANGELOG.md'
       - '.github/workflows/deploy-docs.yml'
 
+# Phase 134.5 (deepsec MEDIUM): least-privilege permissions. The deploy
+# job is the ONLY step that needs `pages: write` + `id-token: write`.
+# Scoping those to that job alone prevents a compromised build step
+# from writing Pages content or minting an OIDC token.
 permissions:
   contents: read
-  pages: write
-  id-token: write
 
 concurrency:
   group: pages
@@ -24,10 +26,12 @@ concurrency:
 jobs:
   build:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2 — Phase 134.5 SHA pin
 
-      - uses: actions/setup-node@v4
+      - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e  # v6.4.0 — Phase 134.5 SHA pin
         with:
           node-version: '22'
           cache: 'npm'
@@ -41,16 +45,19 @@ jobs:
         working-directory: docs-site
         run: npm run build
 
-      - uses: actions/upload-pages-artifact@v3
+      - uses: actions/upload-pages-artifact@fc324d3547104276b827a68afc52ff2a11cc49c9  # v5.0.0 — Phase 134.5 SHA pin
         with:
           path: docs-site/dist
 
   deploy:
     needs: build
     runs-on: ubuntu-latest
+    permissions:
+      pages: write
+      id-token: write
     environment:
       name: github-pages
       url: ${{ steps.deployment.outputs.page_url }}
     steps:
-      - uses: actions/deploy-pages@v4
+      - uses: actions/deploy-pages@cd2ce8fcbc39b97be8ca5fce6e763baed58fa128  # v5.0.0 — Phase 134.5 SHA pin
         id: deployment
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,44 @@ All notable changes to rn-dev-agent will be documented in this file.
 
 Format follows [Keep a Changelog](https://keepachangelog.com/).
 
+## [0.44.35] — 2026-05-12
+
+### Fixed (Phase 134.5 — workflow + correctness sweep, closes 3 MEDIUM + 2 BUG)
+
+- **GitHub Actions pinned to immutable commit SHAs.** Both `ci.yml`
+  and `deploy-docs.yml` previously used mutable `@v4` tags — a moved
+  tag (or compromised maintainer account) would silently substitute
+  different code on the next run. Now pinned to:
+  - `actions/checkout@de0fac2e…` (v6.0.2)
+  - `actions/setup-node@48b55a01…` (v6.4.0)
+  - `actions/upload-pages-artifact@fc324d35…` (v5.0.0)
+  - `actions/deploy-pages@cd2ce8fc…` (v5.0.0)
+
+  Per [GitHub's official security-hardening guide](https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions):
+  "Pinning an action to a full-length commit SHA is currently the only
+  way to use an action as an immutable release."
+- **`deploy-docs.yml` per-job least-privilege permissions.** Previously
+  `pages: write` + `id-token: write` were granted at the workflow level,
+  so the `build` job (which only needs `contents: read`) had Pages-write
+  and OIDC capability it didn't use. Those permissions are now scoped
+  to the `deploy` job only.
+- **`maestro_test_all`'s `pattern` arg now guards against regex DoS.**
+  Caller-supplied `pattern` is length-capped (256 chars) and
+  RegExp construction is try/catch wrapped; on any error, discovery
+  proceeds without filtering rather than crashing.
+- **`cdp_connect`'s already-connected `bundleId` check uses
+  word-boundary matching** instead of `includes()`. Prevents
+  false-positive "already connected" when the live target is e.g.
+  `com.example.app-test` and the caller asked for `com.example.app`.
+
+### Internal
+
+- Workflows: SHA-pin comments include the resolved version name
+  (`# v6.0.2`) so Dependabot can read both and bump them together.
+- `tools/connection.ts` bundleId match uses a regex with non-bundle-id
+  boundary characters (`[^A-Za-z0-9._-]`) — bundle IDs share `.` and
+  `-` with their surrounding context, so `\b` alone wouldn't work.
+
 ## [0.44.34] — 2026-05-12
 
 ### Fixed (Phase 134.4 — CDP multiplexer trust boundary, closes 1 HIGH)
diff --git a/scripts/cdp-bridge/dist/tools/connection.js b/scripts/cdp-bridge/dist/tools/connection.js
@@ -11,8 +11,18 @@ export function createConnectHandler(getClient, setClient, createClient) {
             const haystack = `${target?.title ?? ''} ${target?.description ?? ''}`.toLowerCase();
             const portMismatch = typeof args.metroPort === 'number' && args.metroPort !== client.metroPort;
             const targetIdMismatch = typeof args.targetId === 'string' && args.targetId.length > 0 && args.targetId !== target?.id;
-            const bundleMismatch = typeof args.bundleId === 'string' && args.bundleId.length > 0
-                && !haystack.includes(args.bundleId.toLowerCase());
+            // Phase 134.5 (deepsec BUG: other-logic-bug): the prior substring
+            // check would treat `com.example.app` as "already connected" when
+            // the actual target is `com.example.app-test` or `com.example.app2`
+            // — anything that contained the bundleId as a substring. Use a
+            // word-boundary check anchored on non-bundle-id characters so
+            // `com.example.app` matches `... com.example.app ...` but not
+            // `... com.example.app-test ...`. Bundle IDs use `[A-Za-z0-9._-]`,
+            // so the boundary must be anything outside that set.
+            const bundleIdLower = typeof args.bundleId === 'string' ? args.bundleId.toLowerCase() : '';
+            const bundleMatched = bundleIdLower.length > 0
+                && new RegExp(`(^|[^A-Za-z0-9._-])${bundleIdLower.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}([^A-Za-z0-9._-]|$)`).test(haystack);
+            const bundleMismatch = typeof args.bundleId === 'string' && args.bundleId.length > 0 && !bundleMatched;
             let platformMismatch = false;
             if (typeof args.platform === 'string' && args.platform.length > 0) {
                 const requestedPlatform = args.platform.toLowerCase();
diff --git a/scripts/cdp-bridge/dist/tools/maestro-test-all.js b/scripts/cdp-bridge/dist/tools/maestro-test-all.js
@@ -18,7 +18,21 @@ function discoverFlows(dir, pattern) {
         .map((f) => join(dir, f))
         .sort();
     if (pattern) {
-        const re = new RegExp(pattern, 'i');
+        // Phase 134.5 (deepsec BUG: regex-dos): a malicious or malformed
+        // `pattern` arg could throw on invalid regex syntax or hang on
+        // catastrophic backtracking (e.g. `(a+)+$` against a long input).
+        // Cap the pattern length and wrap construction; on any error,
+        // skip filtering rather than crash discovery.
+        if (pattern.length > 256) {
+            return yamls;
+        }
+        let re;
+        try {
+            re = new RegExp(pattern, 'i');
+        }
+        catch {
+            return yamls;
+        }
         return yamls.filter((f) => re.test(f));
     }
     return yamls;
diff --git a/scripts/cdp-bridge/package.json b/scripts/cdp-bridge/package.json
@@ -1,6 +1,6 @@
 {
   "name": "rn-dev-agent-cdp",
-  "version": "0.38.29",
+  "version": "0.38.30",
   "type": "module",
   "main": "dist/index.js",
   "scripts": {
diff --git a/scripts/cdp-bridge/src/tools/connection.ts b/scripts/cdp-bridge/src/tools/connection.ts
@@ -36,8 +36,18 @@ export function createConnectHandler(
 
       const portMismatch = typeof args.metroPort === 'number' && args.metroPort !== client.metroPort;
       const targetIdMismatch = typeof args.targetId === 'string' && args.targetId.length > 0 && args.targetId !== target?.id;
-      const bundleMismatch = typeof args.bundleId === 'string' && args.bundleId.length > 0
-        && !haystack.includes(args.bundleId.toLowerCase());
+      // Phase 134.5 (deepsec BUG: other-logic-bug): the prior substring
+      // check would treat `com.example.app` as "already connected" when
+      // the actual target is `com.example.app-test` or `com.example.app2`
+      // — anything that contained the bundleId as a substring. Use a
+      // word-boundary check anchored on non-bundle-id characters so
+      // `com.example.app` matches `... com.example.app ...` but not
+      // `... com.example.app-test ...`. Bundle IDs use `[A-Za-z0-9._-]`,
+      // so the boundary must be anything outside that set.
+      const bundleIdLower = typeof args.bundleId === 'string' ? args.bundleId.toLowerCase() : '';
+      const bundleMatched = bundleIdLower.length > 0
+        && new RegExp(`(^|[^A-Za-z0-9._-])${bundleIdLower.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}([^A-Za-z0-9._-]|$)`).test(haystack);
+      const bundleMismatch = typeof args.bundleId === 'string' && args.bundleId.length > 0 && !bundleMatched;
       let platformMismatch = false;
       if (typeof args.platform === 'string' && args.platform.length > 0) {
         const requestedPlatform = args.platform.toLowerCase();
diff --git a/scripts/cdp-bridge/src/tools/maestro-test-all.ts b/scripts/cdp-bridge/src/tools/maestro-test-all.ts
@@ -40,7 +40,20 @@ function discoverFlows(dir: string, pattern?: string): string[] {
     .sort();
 
   if (pattern) {
-    const re = new RegExp(pattern, 'i');
+    // Phase 134.5 (deepsec BUG: regex-dos): a malicious or malformed
+    // `pattern` arg could throw on invalid regex syntax or hang on
+    // catastrophic backtracking (e.g. `(a+)+$` against a long input).
+    // Cap the pattern length and wrap construction; on any error,
+    // skip filtering rather than crash discovery.
+    if (pattern.length > 256) {
+      return yamls;
+    }
+    let re: RegExp;
+    try {
+      re = new RegExp(pattern, 'i');
+    } catch {
+      return yamls;
+    }
     return yamls.filter((f) => re.test(f));
   }
   return yamls;
diff --git a/scripts/cdp-bridge/test/unit/cdp-001-connect-filters.test.js b/scripts/cdp-bridge/test/unit/cdp-001-connect-filters.test.js
@@ -111,3 +111,50 @@ test('CDP-001: multi-mismatch (targetId + bundleId) only disconnects once', asyn
   await h.handler({ targetId: 'new', bundleId: 'com.other' });
   assert.equal(h.counts().disconnects, 1, 'should not double-disconnect on multi-filter mismatch');
 });
+
+// ── Phase 134.5 (deepsec BUG: substring bundleId match) ────────────
+// The prior `haystack.includes(bundleId)` would treat `com.old.app`
+// as "already connected" when the live target was actually
+// `com.old.app-test` or `com.old.app2`. The fix uses a regex with
+// non-bundle-id boundary characters so the match is anchored.
+
+test('Phase 134.5: bundleId `com.old.app` does NOT match haystack containing `com.old.app-test` (regression)', async () => {
+  // Live target is com.old.app-test, caller asks for com.old.app.
+  // Substring match would say "already connected" wrongly.
+  const h = buildHarness({
+    id: 'page1', title: 'com.old.app-test (Hermes)', vm: 'Hermes',
+    description: 'com.old.app-test', platform: 'ios',
+    webSocketDebuggerUrl: 'ws://127.0.0.1:8081/debugger/page1',
+  });
+  const result = await h.handler({ bundleId: 'com.old.app' });
+  const data = JSON.parse(result.content[0].text);
+  // Bundle mismatch should trigger reconnect, NOT alreadyConnected
+  assert.notEqual(data.data?.alreadyConnected, true, 'must NOT short-circuit on substring false-positive');
+  assert.equal(h.counts().disconnects, 1, 'should reconnect to fetch the correct target');
+});
+
+test('Phase 134.5: bundleId `com.old.app` does NOT match haystack containing `com.old.app2` (regression)', async () => {
+  const h = buildHarness({
+    id: 'page1', title: 'com.old.app2 (Hermes)', vm: 'Hermes',
+    description: 'com.old.app2', platform: 'ios',
+    webSocketDebuggerUrl: 'ws://127.0.0.1:8081/debugger/page1',
+  });
+  const result = await h.handler({ bundleId: 'com.old.app' });
+  const data = JSON.parse(result.content[0].text);
+  assert.notEqual(data.data?.alreadyConnected, true);
+  assert.equal(h.counts().disconnects, 1);
+});
+
+test('Phase 134.5: bundleId match still works when surrounded by punctuation (positive parity)', async () => {
+  // Haystack with the bundleId as a standalone token bordered by
+  // whitespace + brackets. The word-boundary check still recognizes it.
+  const h = buildHarness({
+    id: 'page1', title: 'React Native [com.old.app] (Hermes)', vm: 'Hermes',
+    description: 'app: com.old.app, target: page1', platform: 'ios',
+    webSocketDebuggerUrl: 'ws://127.0.0.1:8081/debugger/page1',
+  });
+  const result = await h.handler({ bundleId: 'com.old.app' });
+  const data = JSON.parse(result.content[0].text);
+  assert.equal(data.data.alreadyConnected, true, 'whole-token match still recognized');
+  assert.equal(h.counts().disconnects, 0);
+});

Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@`
`9`	`9`	`{`
`10`	`10`	`"name": "rn-dev-agent",`
`11`	`11`	`"description": "AI agent that fully tests React Native features on simulator/emulator — navigates the app, verifies UI, walks user flows, and confirms internal state.",`
`12`		`- "version": "0.44.34",`
	`12`	`+ "version": "0.44.35",`
`13`	`13`	`"source": "./",`
`14`	`14`	`"category": "mobile-development",`
`15`	`15`	`"homepage": "https://github.com/Lykhoyda/rn-dev-agent"`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "rn-dev-agent",`
`3`		`- "version": "0.44.34",`
	`3`	`+ "version": "0.44.35",`
`4`	`4`	`"description": "AI agent that fully tests React Native features on simulator/emulator — navigates the app, verifies UI, walks user flows, and confirms internal state.",`
`5`	`5`	`"author": {`
`6`	`6`	`"name": "Anton Lykhoyda",`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "rn-dev-agent-cdp",`
`3`		`- "version": "0.38.29",`
	`3`	`+ "version": "0.38.30",`
`4`	`4`	`"type": "module",`
`5`	`5`	`"main": "dist/index.js",`
`6`	`6`	`"scripts": {`