Merge pull request #32 from ownpilot/security/rce-and-high-fixes

security: remediate Critical RCEs + High/Medium findings (scan 2026-06-01)

Author: ersinkoc

.github/workflows/ci.yml

@@ -28,18 +28,18 @@ jobs:
         node-version: [22]
 
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
 
-      - uses: pnpm/action-setup@v4
+      - uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4
 
       - name: Use Node.js ${{ matrix.node-version }}
-        uses: actions/setup-node@v4
+        uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
         with:
           node-version: ${{ matrix.node-version }}
           cache: 'pnpm'
 
       - name: Cache Turbo
-        uses: actions/cache@v4
+        uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
         with:
           path: .turbo
           key: turbo-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }}-${{ github.sha }}
@@ -102,12 +102,12 @@ jobs:
           --health-retries 5
 
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
 
-      - uses: pnpm/action-setup@v4
+      - uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4
 
       - name: Use Node.js 22
-        uses: actions/setup-node@v4
+        uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
         with:
           node-version: 22
           cache: 'pnpm'

.github/workflows/deploy-website.yml

@@ -25,10 +25,10 @@ jobs:
         working-directory: website
     steps:
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
 
       - name: Setup Node.js
-        uses: actions/setup-node@v4
+        uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
         with:
           node-version: '22'
           cache: 'npm'
@@ -41,10 +41,10 @@ jobs:
         run: npm run build
 
       - name: Setup Pages
-        uses: actions/configure-pages@v5
+        uses: actions/configure-pages@983d7736d9b0ae728b81ab479565c72886d7745b # v5
 
       - name: Upload artifact
-        uses: actions/upload-pages-artifact@v3
+        uses: actions/upload-pages-artifact@56afc609e74202658d3ffba0e8f6dda462b719fa # v3
         with:
           path: website/dist
 
@@ -57,4 +57,4 @@ jobs:
     steps:
       - name: Deploy to GitHub Pages
         id: deployment
-        uses: actions/deploy-pages@v4
+        uses: actions/deploy-pages@d6db90164ac5ed86f2b6aed7e0febac5b3c0c03e # v4

.github/workflows/release.yml

@@ -19,20 +19,20 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
         with:
           fetch-depth: 0
 
-      - uses: pnpm/action-setup@v4
+      - uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4
 
       - name: Use Node.js 22
-        uses: actions/setup-node@v4
+        uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
         with:
           node-version: 22
           cache: 'pnpm'
 
       - name: Cache Turbo
-        uses: actions/cache@v4
+        uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4
         with:
           path: .turbo
           key: turbo-${{ runner.os }}-${{ hashFiles('pnpm-lock.yaml') }}-${{ github.sha }}
@@ -59,18 +59,18 @@ jobs:
         run: TURBO_FORCE=true pnpm test
 
       - name: Set up Docker Buildx
-        uses: docker/setup-buildx-action@v3
+        uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3
 
       - name: Login to GHCR
-        uses: docker/login-action@v3
+        uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3
         with:
           registry: ghcr.io
           username: ${{ github.actor }}
           password: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Docker metadata
         id: meta
-        uses: docker/metadata-action@v5
+        uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5
         with:
           images: ghcr.io/ownpilot/ownpilot
           tags: |
@@ -79,7 +79,7 @@ jobs:
             type=raw,value=latest,enable=${{ !contains(github.ref_name, '-') }}
 
       - name: Build and push Docker image
-        uses: docker/build-push-action@v6
+        uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6
         with:
           context: .
           push: true
@@ -90,6 +90,6 @@ jobs:
           cache-to: type=gha,mode=max
 
       - name: Create GitHub Release
-        uses: softprops/action-gh-release@v2
+        uses: softprops/action-gh-release@3bb12739c298aeb8a4eeaf626c5b8d85266b0e65 # v2
         with:
           generate_release_notes: true

docker-compose.db.yml

@@ -22,6 +22,11 @@ services:
       - '127.0.0.1:${POSTGRES_PORT:-25432}:5432'
     environment:
       POSTGRES_USER: ${POSTGRES_USER:-ownpilot}
+      # SECURITY (DOCK-003): `ownpilot_secret` is a committed, publicly-known
+      # fallback intended ONLY for local development. The port above is bound to
+      # 127.0.0.1, so the DB is never exposed beyond this host. For any shared,
+      # remote, or production use you MUST set POSTGRES_PASSWORD in your .env to
+      # a strong secret (the gateway reads the same value).
       POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-ownpilot_secret}
       POSTGRES_DB: ${POSTGRES_DB:-ownpilot}
     volumes:

docker-compose.yml

@@ -86,8 +86,13 @@ services:
     container_name: ownpilot-mosquitto
     restart: unless-stopped
     ports:
-      - '${MQTT_PORT:-1883}:1883'
-      - '${MQTT_WS_PORT:-9001}:9001'
+      # SECURITY (DOCK-004/IAC-002): bind to IPv4 loopback only. The broker
+      # ships with allow_anonymous=true, so exposing these ports on 0.0.0.0
+      # would give any host on the LAN full read/write on all MQTT topics
+      # (including edge-device control + agent messaging). Other containers
+      # still reach the broker over the compose network by service name.
+      - '127.0.0.1:${MQTT_PORT:-1883}:1883'
+      - '127.0.0.1:${MQTT_WS_PORT:-9001}:9001'
     volumes:
       - ./packages/gateway/docker/mosquitto/config:/mosquitto/config:ro
       - mosquitto-data:/mosquitto/data

packages/core/src/credentials/index.ts

@@ -218,7 +218,10 @@ function deriveKey(masterKey: string, salt?: Buffer): Buffer {
   if (salt) {
     return pbkdf2Sync(masterKey, salt, PBKDF2_ITERATIONS, 32, 'sha256');
   }
-  // Legacy fallback — single SHA-256 (no brute-force resistance)
+  // Legacy fallback — single SHA-256 (no brute-force resistance). Retained
+  // ONLY to read pre-salt entries; those are re-encrypted with PBKDF2+salt on
+  // first read via migrateLegacyEntryIfNeeded (CRYPTO-004), so this path is
+  // self-healing and a no-salt entry should not persist past one read.
   return createHash('sha256').update(masterKey).digest();
 }
 
@@ -336,6 +339,25 @@ export class UserCredentialStore {
     return id;
   }
 
+  /**
+   * CRYPTO-004: lazily upgrade legacy (no-salt, single-SHA-256) entries to the
+   * PBKDF2+salt scheme the first time they are successfully decrypted, so the
+   * weak O(1) key derivation never survives a read. Non-fatal on failure — a
+   * migration error must never break a credential read.
+   */
+  private async migrateLegacyEntryIfNeeded(
+    entry: CredentialEntry,
+    plaintext: string
+  ): Promise<void> {
+    if (entry.salt) return;
+    try {
+      const { encrypted, iv, salt } = encryptValue(plaintext, this.config.encryptionKey);
+      await this.backend.set({ ...entry, encryptedValue: encrypted, iv, salt });
+    } catch {
+      // best-effort; keep serving the decrypted value
+    }
+  }
+
   /**
    * Get a credential for use
    */
@@ -359,6 +381,9 @@ export class UserCredentialStore {
       entry.salt
     );
 
+    // CRYPTO-004: upgrade legacy no-salt entries on read.
+    await this.migrateLegacyEntryIfNeeded(entry, value);
+
     // Update usage
     await this.updateUsage(entry);
 
@@ -394,6 +419,9 @@ export class UserCredentialStore {
       entry.salt
     );
 
+    // CRYPTO-004: upgrade legacy no-salt entries on read.
+    await this.migrateLegacyEntryIfNeeded(entry, value);
+
     // Update usage
     await this.updateUsage(entry);
 

packages/core/src/sandbox/code-validator.test.ts

@@ -122,6 +122,24 @@ describe('new dangerous patterns', () => {
     expect(result.valid).toBe(false);
     expect(result.errors).toContain('Atomics is not allowed (timing attack vector)');
   });
+
+  // RCE-001 regression: the constructor-walk escape relies on splitting the
+  // word "constructor" across string-concat to evade the literal-token check.
+  // Every single split point must be caught. (Defense-in-depth; the VM sandbox
+  // itself no longer injects host objects, so even a 3-way split cannot escape.)
+  it.each([
+    `Math['constructo'+'r']('return process')`,
+    `Math['construct'+'or']('x')`,
+    `Math['con'+'structor']('x')`,
+    `Math['constr'+'uctor']('x')`,
+    `Math['constru'+'ctor']('x')`,
+    `obj['co'+'nstructor']`,
+    `obj['constructo' + 'r']`,
+  ])('blocks split-string constructor access: %s', (code) => {
+    const result = validateToolCode(code);
+    expect(result.valid).toBe(false);
+    expect(result.errors).toContain('string-concat constructor access is not allowed');
+  });
 });
 
 // ---------------------------------------------------------------------------

packages/core/src/sandbox/code-validator.ts

@@ -106,15 +106,21 @@ export const DANGEROUS_CODE_PATTERNS: ReadonlyArray<CodeValidationPattern> = [
     pattern: /[.[]\s*['"]?\s*constructor\b/,
     message: 'constructor property access is not allowed',
   },
-  // String-concat bypass — `obj['construct'+'or']` evades the literal-token
+  // String-concat bypass — `obj['constructo'+'r']` evades the literal-token
   // check above. Legitimate code does not assemble the word "constructor"
-  // from fragments. Catch the common splits explicitly.
+  // from fragments. These two patterns cover EVERY single split point of the
+  // word (left prefix immediately followed by `+`, or `+` immediately followed
+  // by a right suffix). NOTE: this is defense-in-depth only — the real fix is
+  // that the VM sandbox no longer injects any host-realm object, so even a
+  // multi-fragment split that evades these regexes can only reach the
+  // context-realm Function constructor, which `codeGeneration.strings:false`
+  // disables. See services/extension/sandbox.ts (RCE-001).
   {
-    pattern: /(['"]construct['"]|['"]constr['"]|['"]con['"]).*\+/,
+    pattern: /['"](?:co|con|cons|const|constr|constru|construc|construct|constructo)['"]\s*\+/,
     message: 'string-concat constructor access is not allowed',
   },
   {
-    pattern: /\+.*?(['"]uctor['"]|['"]ructor['"]|['"]tor['"]|['"]structor['"])/,
+    pattern: /\+\s*['"](?:r|or|tor|ctor|uctor|ructor|tructor|structor|nstructor|onstructor)['"]/,
     message: 'string-concat constructor access is not allowed',
   },
   { pattern: /\bgetPrototypeOf\b/, message: 'getPrototypeOf is not allowed' },

packages/gateway/src/channels/service-impl.ts

@@ -6,7 +6,7 @@
  * through EventBus, handles verification, and manages sessions.
  */
 
-import { timingSafeEqual } from 'node:crypto';
+import { timingSafeEqual, randomInt } from 'node:crypto';
 import {
   type IChannelService,
   type ChannelOutgoingMessage,
@@ -1430,8 +1430,10 @@ export class ChannelServiceImpl implements IChannelService {
     senderUserId: string,
     _ownerUserId: string
   ): Promise<string> {
-    // Generate 6-digit code
-    const code = String(Math.floor(100000 + Math.random() * 900000));
+    // Generate 6-digit code. SECURITY (EXPOSE-004): use a CSPRNG — Math.random()
+    // is a non-cryptographic PRNG whose internal state can be recovered from a
+    // few observed outputs, making pairing codes predictable/brute-forceable.
+    const code = String(randomInt(100000, 1000000));
 
     // Store in verification_tokens
     await this.dmPairingRequests.create({

packages/gateway/src/db/repositories/logs.test.ts

@@ -178,6 +178,18 @@ describe('LogsRepository', () => {
       expect(params[18]).toBeNull(); // userAgent
     });
 
+    it('truncates an oversized error stack to the cap (EXPOSE-001)', async () => {
+      mockAdapter.execute.mockResolvedValueOnce({ changes: 1 });
+      mockAdapter.queryOne.mockResolvedValueOnce(makeLogRow());
+
+      const hugeStack = 'Error: boom\n' + '    at frame\n'.repeat(5000);
+      await repo.log({ type: 'chat', errorStack: hugeStack });
+
+      const params = mockAdapter.execute.mock.calls[0]![1] as unknown[];
+      expect((params[16] as string).length).toBe(2000); // persisted stack capped
+      expect(params[16]).toBe(hugeStack.slice(0, 2000));
+    });
+
     it('should return a fallback log entry when insert fails', async () => {
       mockAdapter.execute.mockRejectedValueOnce(new Error('DB connection lost'));