Codex/security coderabbit fixes

.deepsec/.gitignore

@@ -6,3 +6,4 @@ data/*/runs/
 data/*/reports/
 findings/
 comment.md
+.tmp/

.deepsec/matchers/android-exported-component.ts

@@ -11,7 +11,7 @@ export const androidExportedComponent: MatcherPlugin = {
     return regexCandidates("android-exported-component", content, [
       {
         regex:
-          /<(activity|activity-alias|service|receiver)\b[\s\S]*?android:exported\s*=\s*"true"[\s\S]*?(?:<\/\1>|\/>)/,
+          /<(activity|activity-alias|service|receiver)\b[\s\S]*?android:exported\s*=\s*(["'])true\2[\s\S]*?(?:<\/\1>|\/>)/,
         label: 'Android component with android:exported="true"',
       },
     ]);

.deepsec/matchers/android-uri-share-without-clipdata.ts

@@ -1,5 +1,13 @@
 import type { CandidateMatch, MatcherPlugin } from "deepsec/config";
-import { candidate, isTestFile } from "./utils.js";
+import {
+  candidate,
+  findBalancedDelimiterEnd,
+  findCallOpeningParenthesis,
+  findEnclosingBlockEnds,
+  isTestFile,
+} from "./utils.js";
+
+const sendAction = /\bIntent\.ACTION_SEND(?:_MULTIPLE)?\b/g;
 
 export const androidUriShareWithoutClipData: MatcherPlugin = {
   slug: "android-uri-share-without-clipdata",
@@ -12,23 +20,63 @@ export const androidUriShareWithoutClipData: MatcherPlugin = {
     if (!content.includes("Intent.ACTION_SEND") && !content.includes("Intent.ACTION_SEND_MULTIPLE")) return [];
     if (!content.includes("Intent.EXTRA_STREAM")) return [];
 
-    const hasReadGrant = content.includes("FLAG_GRANT_READ_URI_PERMISSION");
-    const hasClipData = /\bclipData\b|ClipData\./.test(content);
-    if (hasReadGrant && hasClipData) return [];
+    return shareScopes(content).flatMap(({ start, text }) => {
+      if (!text.includes("Intent.EXTRA_STREAM")) return [];
 
-    const sendIndex = content.indexOf("Intent.ACTION_SEND");
-    const sendMultipleIndex = content.indexOf("Intent.ACTION_SEND_MULTIPLE");
-    const index = [sendIndex, sendMultipleIndex].filter((value) => value >= 0).sort((a, b) => a - b)[0] ?? 0;
+      const hasReadGrant = text.includes("FLAG_GRANT_READ_URI_PERMISSION");
+      const hasClipData = /\bclipData\b|ClipData\./.test(text);
+      if (hasReadGrant && hasClipData) return [];
 
-    return [
-      candidate(
+      return candidate(
         "android-uri-share-without-clipdata",
         content,
-        index,
+        start,
         hasReadGrant
           ? "EXTRA_STREAM content URI share without ClipData"
           : "EXTRA_STREAM content URI share without FLAG_GRANT_READ_URI_PERMISSION",
-      ),
-    ];
+      );
+    });
   },
 };
+
+function shareScopes(content: string): Array<{ start: number; text: string }> {
+  const starts = [...content.matchAll(sendAction)].map((match) => match.index ?? 0);
+  return starts.map((start, index) => {
+    const nextShareStart = starts[index + 1] ?? content.length;
+    const end = findShareScopeEnd(content, start, nextShareStart);
+    return { start, text: content.slice(start, end) };
+  });
+}
+
+function findShareScopeEnd(content: string, start: number, nextShareStart: number): number {
+  const builderEnd = findIntentBuilderEnd(content, start);
+  if (builderEnd !== null) {
+    const boundedBuilderEnd = Math.min(nextShareStart, builderEnd + 1);
+    if (content.slice(start, boundedBuilderEnd).includes("Intent.EXTRA_STREAM")) {
+      return boundedBuilderEnd;
+    }
+  }
+
+  for (const enclosingBlockEnd of findEnclosingBlockEnds(content, start)) {
+    const boundedBlockEnd = Math.min(nextShareStart, enclosingBlockEnd + 1);
+    if (content.slice(start, boundedBlockEnd).includes("Intent.EXTRA_STREAM")) {
+      return boundedBlockEnd;
+    }
+  }
+
+  return nextShareStart;
+}
+
+function findIntentBuilderEnd(content: string, actionStart: number): number | null {
+  const openParenthesis = findCallOpeningParenthesis(content, "Intent", actionStart);
+  if (openParenthesis === null) return null;
+
+  const closeParenthesis = findBalancedDelimiterEnd(content, openParenthesis, "(", ")");
+  if (closeParenthesis === null) return null;
+
+  const builderMatch = /^\s*\.(?:apply|also|let|run)\s*\{/.exec(content.slice(closeParenthesis + 1));
+  if (builderMatch === null) return null;
+
+  const openBrace = closeParenthesis + 1 + builderMatch[0].lastIndexOf("{");
+  return findBalancedDelimiterEnd(content, openBrace, "{", "}");
+}

.deepsec/matchers/fileprovider-broad-path.ts

@@ -14,7 +14,8 @@ export const fileproviderBroadPath: MatcherPlugin = {
       { regex: /<root-path\b[^>]*>/i, label: "FileProvider root-path exposes filesystem root" },
       { regex: /<external-path\b[^>]*>/i, label: "FileProvider external-path exposes shared storage" },
       {
-        regex: /<(?:files-path|cache-path|external-files-path|external-cache-path)\b[^>]*\bpath\s*=\s*"[\s.\/]*"[^>]*>/i,
+        regex:
+          /<(?:files-path|cache-path|external-files-path|external-cache-path)\b[^>]*\bpath\s*=\s*(["'])[\s.\/]*\1[^>]*>/i,
         label: 'FileProvider path="." or equivalent broad directory',
       },
     ]);

.deepsec/matchers/foreground-audio-service-start.ts

@@ -17,7 +17,7 @@ export const foregroundAudioServiceStart: MatcherPlugin = {
         label: "Audio session start from foreground service",
       },
       {
-        regex: /ServiceCompat\.startForeground\s*\(/,
+        regex: /\b(?:ServiceCompat\.)?startForeground\s*\(/,
         label: "Foreground promotion call",
       },
     ]);

.deepsec/matchers/health-connect-sensitive-flow.ts

@@ -9,7 +9,13 @@ export const healthConnectSensitiveFlow: MatcherPlugin = {
   filePatterns: ["app/src/main/**/*.kt", "app/src/main/AndroidManifest.xml"],
   match(content, filePath): CandidateMatch[] {
     if (isTestFile(filePath)) return [];
-    if (!/HealthConnect|health\.|HeartRate|ExerciseSession|WRITE_EXERCISE|READ_HEART_RATE/.test(content)) return [];
+    if (
+      !/HealthConnect|health\.|HeartRate|ExerciseSession|WRITE_EXERCISE|READ_HEART_RATE|HealthPermission\.get(?:Write|Read)Permission|writeNoiseDose/.test(
+        content,
+      )
+    ) {
+      return [];
+    }
 
     return regexCandidates("health-connect-sensitive-flow", content, [
       { regex: /WRITE_EXERCISE|HealthPermission\.getWritePermission/, label: "Health Connect write permission" },

.deepsec/matchers/security-matchers.test.ts

@@ -0,0 +1,193 @@
+import assert from "node:assert/strict";
+import { test } from "node:test";
+import { androidExportedComponent } from "./android-exported-component.js";
+import { androidUriShareWithoutClipData } from "./android-uri-share-without-clipdata.js";
+import { fileproviderBroadPath } from "./fileprovider-broad-path.js";
+import { foregroundAudioServiceStart } from "./foreground-audio-service-start.js";
+import { healthConnectSensitiveFlow } from "./health-connect-sensitive-flow.js";
+import { sensitiveAndroidLog } from "./sensitive-android-log.js";
+
+test("health matcher gates on permission helpers and noise-dose writes", () => {
+  const writePermission = healthConnectSensitiveFlow.match(
+    "HealthPermission.getWritePermission(ExerciseSessionRecord::class)",
+    "app/src/main/java/com/dbcheck/app/HealthPermissions.kt",
+  );
+  const noiseDose = healthConnectSensitiveFlow.match(
+    "suspend fun sync() = writeNoiseDose(report)",
+    "app/src/main/java/com/dbcheck/app/HealthSync.kt",
+  );
+
+  assert.ok(writePermission.some((match) => match.matchedPattern === "Health Connect write permission"));
+  assert.ok(noiseDose.some((match) => match.matchedPattern === "Noise-dose write flow"));
+});
+
+test("sensitive log matcher covers multiline calls", () => {
+  const matches = sensitiveAndroidLog.match(
+    `Log.i(
+      TAG,
+      "Exported session URI: $uri",
+    )`,
+    "app/src/main/java/com/dbcheck/app/ExportLogger.kt",
+  );
+
+  assert.equal(matches.length, 1);
+});
+
+test("sensitive log matcher does not consume a later Kotlin statement", () => {
+  const matches = sensitiveAndroidLog.match(
+    `Log.i(TAG, "sync complete")
+val exportedUri = uri`,
+    "app/src/main/java/com/dbcheck/app/HealthSync.kt",
+  );
+
+  assert.deepEqual(matches, []);
+});
+
+test("sensitive log matcher ignores closing parentheses inside Kotlin comments", () => {
+  const matches = sensitiveAndroidLog.match(
+    `Log.i(
+      TAG, // )
+      "Exported session URI: $uri",
+    )
+Log.w(
+  TAG, /* ) */
+  "Backup file URI: $uri",
+)`,
+    "app/src/main/java/com/dbcheck/app/ExportLogger.kt",
+  );
+
+  assert.equal(matches.length, 2);
+});
+
+test("foreground matcher covers direct and ServiceCompat promotion", () => {
+  const direct = foregroundAudioServiceStart.match(
+    "class MeasurementForegroundService { fun promote() = startForeground(ID, notification) }",
+    "app/src/main/java/com/dbcheck/app/MeasurementForegroundService.kt",
+  );
+  const compatible = foregroundAudioServiceStart.match(
+    "class MeasurementForegroundService { fun promote() = ServiceCompat.startForeground(this, ID, notification, 0) }",
+    "app/src/main/java/com/dbcheck/app/MeasurementForegroundService.kt",
+  );
+
+  assert.equal(direct.length, 1);
+  assert.equal(compatible.length, 1);
+});
+
+test("FileProvider matcher covers single-quoted broad paths", () => {
+  const matches = fileproviderBroadPath.match(
+    "<paths><cache-path name='cache' path='.'/></paths>",
+    "app/src/main/res/xml/file_paths.xml",
+  );
+
+  assert.equal(matches.length, 1);
+});
+
+test("exported component matcher covers single-quoted attributes", () => {
+  const matches = androidExportedComponent.match(
+    "<manifest><application><service android:name='.SyncService' android:exported='true'/></application></manifest>",
+    "app/src/main/AndroidManifest.xml",
+  );
+
+  assert.equal(matches.length, 1);
+});
+
+test("URI share matcher evaluates each share construction independently", () => {
+  const content = `
+fun safe(uri: Uri): Intent =
+  Intent(Intent.ACTION_SEND).apply {
+    putExtra(Intent.EXTRA_STREAM, uri)
+    clipData = ClipData.newUri(resolver, "safe", uri)
+    addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
+  }
+
+fun unsafe(uri: Uri): Intent =
+  Intent(Intent.ACTION_SEND).apply {
+    putExtra(Intent.EXTRA_STREAM, uri)
+  }
+`;
+
+  const matches = androidUriShareWithoutClipData.match(
+    content,
+    "app/src/main/java/com/dbcheck/app/ShareFactory.kt",
+  );
+
+  assert.equal(matches.length, 1);
+  assert.equal(
+    matches[0]?.matchedPattern,
+    "EXTRA_STREAM content URI share without FLAG_GRANT_READ_URI_PERMISSION",
+  );
+});
+
+test("URI share matcher reports a missing ClipData in an otherwise granted scope", () => {
+  const matches = androidUriShareWithoutClipData.match(
+    `fun share(uri: Uri) = Intent(Intent.ACTION_SEND_MULTIPLE).apply {
+      putParcelableArrayListExtra(Intent.EXTRA_STREAM, arrayListOf(uri))
+      addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
+    }`,
+    "app/src/main/java/com/dbcheck/app/ShareFactory.kt",
+  );
+
+  assert.equal(matches.length, 1);
+  assert.equal(matches[0]?.matchedPattern, "EXTRA_STREAM content URI share without ClipData");
+});
+
+test("URI share matcher bounds an unsafe share to its builder block", () => {
+  const matches = androidUriShareWithoutClipData.match(
+    `fun share(uri: Uri): Intent {
+      val intent = Intent(Intent.ACTION_SEND).apply {
+        putExtra(Intent.EXTRA_STREAM, uri)
+      }
+      val unrelatedClipData = ClipData.newPlainText("preview", "text")
+      val unrelatedFlags = Intent.FLAG_GRANT_READ_URI_PERMISSION
+      return intent
+    }`,
+    "app/src/main/java/com/dbcheck/app/ShareFactory.kt",
+  );
+
+  assert.equal(matches.length, 1);
+  assert.equal(
+    matches[0]?.matchedPattern,
+    "EXTRA_STREAM content URI share without FLAG_GRANT_READ_URI_PERMISSION",
+  );
+});
+
+test("URI share matcher continues from a nested constructor to its owning function", () => {
+  const matches = androidUriShareWithoutClipData.match(
+    `fun share(uri: Uri): Intent {
+      val intent = run {
+        Intent(Intent.ACTION_SEND)
+      }
+      intent.putExtra(Intent.EXTRA_STREAM, uri)
+      return intent
+    }`,
+    "app/src/main/java/com/dbcheck/app/ShareFactory.kt",
+  );
+
+  assert.equal(matches.length, 1);
+  assert.equal(
+    matches[0]?.matchedPattern,
+    "EXTRA_STREAM content URI share without FLAG_GRANT_READ_URI_PERMISSION",
+  );
+});
+
+test("URI share matcher recognizes constructor trivia before the opening parenthesis", () => {
+  const matches = androidUriShareWithoutClipData.match(
+    `fun share(uri: Uri): Intent {
+      val intent = Intent /* share constructor */ (
+        Intent.ACTION_SEND
+      ).apply {
+        putExtra(Intent.EXTRA_STREAM, uri)
+      }
+      val unrelatedClipData = ClipData.newPlainText("preview", "text")
+      val unrelatedFlags = Intent.FLAG_GRANT_READ_URI_PERMISSION
+      return intent
+    }`,
+    "app/src/main/java/com/dbcheck/app/ShareFactory.kt",
+  );
+
+  assert.equal(matches.length, 1);
+  assert.equal(
+    matches[0]?.matchedPattern,
+    "EXTRA_STREAM content URI share without FLAG_GRANT_READ_URI_PERMISSION",
+  );
+});

.deepsec/matchers/sensitive-android-log.ts

@@ -1,8 +1,10 @@
 import type { CandidateMatch, MatcherPlugin } from "deepsec/config";
-import { isTestFile, regexCandidates } from "./utils.js";
+import { candidate, findBalancedDelimiterEnd, isTestFile } from "./utils.js";
 
 const sensitiveWords =
   "(?:session|sessionId|measurement|decibel|dbWeighted|dbcheck\\.db|audio|microphone|recording|sample|billing|purchase|token|backup|restore|health|heart|threshold|hearing|csv|pdf|export|uri|fileprovider)";
+const sensitiveWordPattern = new RegExp(sensitiveWords, "i");
+const logCallStart = /\b(?:Log|android\.util\.Log)\.(?:v|d|i|w|e)\s*\(/g;
 
 export const sensitiveAndroidLog: MatcherPlugin = {
   slug: "sensitive-android-log",
@@ -13,14 +15,13 @@ export const sensitiveAndroidLog: MatcherPlugin = {
   match(content, filePath): CandidateMatch[] {
     if (isTestFile(filePath)) return [];
 
-    return regexCandidates("sensitive-android-log", content, [
-      {
-        regex: new RegExp(
-          String.raw`\b(?:Log|android\.util\.Log)\.(?:v|d|i|w|e)\s*\([^;\n]*${sensitiveWords}[^;\n]*\)`,
-          "i",
-        ),
-        label: "Sensitive term in Android log call",
-      },
-    ]);
+    return [...content.matchAll(logCallStart)].flatMap((match) => {
+      const start = match.index ?? 0;
+      const openParenthesis = start + match[0].lastIndexOf("(");
+      const end = findBalancedDelimiterEnd(content, openParenthesis, "(", ")");
+      if (end === null || !sensitiveWordPattern.test(content.slice(openParenthesis + 1, end))) return [];
+
+      return candidate("sensitive-android-log", content, start, "Sensitive term in Android log call");
+    });
   },
 };