Skip to content

[codex] Guard public repo sensitive content - #497

Merged
100yenadmin merged 6 commits into
mainfrom
codex/public-security-audit-20260630
Jul 1, 2026
Merged

[codex] Guard public repo sensitive content#497
100yenadmin merged 6 commits into
mainfrom
codex/public-security-audit-20260630

Conversation

@100yenadmin

@100yenadmin 100yenadmin commented Jun 30, 2026

Copy link
Copy Markdown
Owner

Summary

  • Removes hardcoded mobile Apple account/team defaults from the public tree and makes iOS build/submit account data environment-backed.
  • Adds scripts/check-public-sensitive-content.js, bun run security:public-scan, tests, and fixture allowlists for public-repo sensitive-content checks.
  • Adds a full-history public security scan workflow with pinned Gitleaks/TruffleHog actions and wires the repo-owned scan into the required PR Code Quality job.

Evidence and tracking

Security boundary

This PR cleans the current tracked tree and adds regression guardrails. It does not claim public history was never exposed. Redacted probes found historical Apple account/team identifiers and secret-shaped historical patterns that require owner rotation/revocation/history-pruning decisions if they correspond to real credentials.

Validation

  • bunx vitest run tests/unit/process/checkPublicSensitiveContent.test.ts
  • bun run security:public-scan
  • node --check scripts/check-public-sensitive-content.js
  • git grep check for removed Apple account/team literals: no current-tree matches
  • bun run format:check
  • ruby YAML parse for changed workflows
  • bun run lint exited 0 with the existing warning backlog
  • git diff --cached --check

Summary by CodeRabbit

  • New Features
    • Added a public security scan that checks for exposed secrets during CI and can be run from the project scripts.
    • Improved mobile build and submit handling with clearer app identity settings and better iOS submission validation.
  • Bug Fixes
    • Expanded packaging checks to accept both placeholder and pinned release bridge manifests when the expected values match.
    • Tightened iOS build/submit behavior to prevent invalid direct-submit usage and missing credentials.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 39 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5d17f20e-f87c-44c2-ab13-05770ec7e9ed

📥 Commits

Reviewing files that changed from the base of the PR and between b365019 and 102fb96.

📒 Files selected for processing (8)
  • .github/workflows/pr-checks.yml
  • .github/workflows/public-security-scan.yml
  • .gitleaks.toml
  • mobile/eas.json
  • mobile/scripts/build.js
  • package.json
  • scripts/security/check-public-sensitive-content.js
  • tests/unit/process/checkPublicSensitiveContent.test.ts
📝 Walkthrough

Walkthrough

Adds a repo-owned public sensitive-content scanner (Node script plus tests), wires it into a new public-security-scan CI workflow alongside Gitleaks and TruffleHog jobs and into pr-checks, adds Gitleaks allowlist config, removes hardcoded Apple credentials from mobile build tooling/config, and updates thin-app smoke Bridge manifest verification.

Changes

Public security audit guardrails

Layer / File(s) Summary
Sensitive content scanner and tests
scripts/security/check-public-sensitive-content.js, tests/unit/process/checkPublicSensitiveContent.test.ts
New scanner detects risky filenames and rule-based content matches (Apple IDs, private keys, API tokens, credential URLs) with allowlisting, exports scanPath/scanRepository/scanText, CLI exits 1 on findings; unit tests cover detection and allowlist behaviors.
npm script and CI wiring
package.json, .github/workflows/public-security-scan.yml, .github/workflows/pr-checks.yml, .gitleaks.toml
Adds security:public-scan npm script, a new workflow running the scanner plus Gitleaks and TruffleHog jobs, a Gitleaks allowlist for test fixtures, and a scan step in the existing Code Quality job.
Thin-app smoke Bridge verification
.github/workflows/pr-checks.yml
Bridge manifest check now accepts a pinned-real Bridge (validated SHA fields, matching requestedSourceRef/sourceCommit) in addition to the diagnostic placeholder, with updated summary wording.
Mobile Apple credential cleanup
mobile/eas.json, mobile/scripts/build.js
Replaces hardcoded appleId/appleTeamId with identity fields in eas.json; build.js requires Apple env vars via a new requireEnv helper, uses projectRoot for paths/cwd, restricts --direct-submit to --local, and adds --profile to submit commands.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Possibly related PRs

  • 100yenadmin/evaOS-GUI#455: Both modify the thin-app macOS ARM64 packaging smoke job's Bridge manifest verification and BRIDGE_SUMMARY output logic in pr-checks.yml.
  • 100yenadmin/evaOS-GUI#470: Both change the macOS Bridge verification logic computing BRIDGE_SUMMARY based on placeholder vs pinned-real manifest states.

Suggested labels: risk:security, kind:integration, evaos

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and matches the PR’s main theme of guarding public sensitive content.
Description check ✅ Passed The description covers summary, related issue, security context, and validation, though it omits the template’s type/test checkboxes and screenshots.
Linked Issues check ✅ Passed The PR removes Apple defaults, adds the repo-owned scanner/tests, and adds public-scan CI, satisfying the linked security-audit requirements.
Out of Scope Changes check ✅ Passed The diff is centered on security scanning, Apple-default cleanup, and CI guardrails; no clearly unrelated changes are evident.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/public-security-audit-20260630

Comment @coderabbitai help to get the list of available commands.

@100yenadmin
100yenadmin marked this pull request as ready for review June 30, 2026 14:17
@100yenadmin

Copy link
Copy Markdown
Owner Author

evaOS-GUI public security audit current-head status

Date: 2026-06-30
Repository: 100yenadmin/evaOS-GUI
Issue: #496
PR: #497
Base: b91c11b
Head: eea0de1

Implemented

  • Removed hardcoded Apple account/team defaults from mobile iOS build config.
  • Made mobile iOS build/submit use env-only Apple account/team configuration.
  • Added repo-owned redacted sensitive-content scanner and security:public-scan script.
  • Added unit coverage for the scanner allowlist/block behavior.
  • Added public security scan workflow with full-history Gitleaks and verified-only TruffleHog.
  • Added repo-owned public scan to PR code-quality checks.
  • Fixed PR thin app smoke to accept either diagnostic Bridge placeholder or pinned real Bridge resource.
  • Enabled Dependabot security updates and disabled stale workflow registry records that no longer had files on main.

Current-head verification

Remaining proof boundary / human decisions

  • Full-history Gitleaks and verified-only TruffleHog passed on the PR workflow.
  • Redacted local all-ref triage still found historical account identifiers and a Google-API-key-shaped value in old source history. Treat as requiring human rotate/revoke/confirm-fake decision before claiming old public history is harmless.
  • Secret-scanning non-provider patterns and validity checks remain disabled by GitHub/account capability after enable attempts.
  • CodeQL/code scanning has no analysis yet.
  • Dependabot alerts are now visible and should be triaged as a separate dependency-security issue.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af9415540c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/check-public-sensitive-content.js Outdated
Comment thread .github/workflows/public-security-scan.yml Outdated
Comment thread .gitleaks.toml Outdated
Comment thread scripts/check-public-sensitive-content.js Outdated
Comment thread scripts/security/check-public-sensitive-content.js
Comment thread mobile/eas.json Outdated

@evaos-code-review-bot evaos-code-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

evaOS ZCode review result for #497 at eea0de1.

Inline comments: 1. High-severity comments: 0. Dropped findings: 1.

Pilot policy: this bot never approves PRs; it requests changes only for validated P0/P1 findings.

Comment thread mobile/scripts/build.js Outdated
@100yenadmin

Copy link
Copy Markdown
Owner Author

Release-planning disposition: keep open and fix before any public release claim. CI is green, but this PR remains review-required with unresolved current-head review threads around scanner secret handling, full-history scanning/allowlists, encrypted key detection, scanner location, EAS submit config, and iOS build-only env requirements.

Planning packet: #480 (comment)

Boundary: this PR is not part of the proof-tagged direct Mac-control private RC; it is a public-release hygiene blocker.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai coderabbitai Bot added evaos evaOS public beta R&D work kind:integration Integration implementation issue risk:security Security, auth, secrets, permission risk labels Jul 1, 2026
@100yenadmin
100yenadmin force-pushed the codex/public-security-audit-20260630 branch from b365019 to 66b6df3 Compare July 1, 2026 08:46
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

coderabbitai[bot]
coderabbitai Bot previously requested changes Jul 1, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.gitleaks.toml (1)

6-35: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Anchor the Gitleaks allowlist path regexes.

  • paths are regexes, so these entries can match any longer path containing the fixture filename fragment; anchor them to the exact test files to keep the suppressions scoped.
Suggested fix
-paths = ['''tests/unit/common/protocolDetector\.test\.ts''']
+paths = ['''^tests/unit/common/protocolDetector\.test\.ts$''']

Apply the same change to the other four paths entries.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.gitleaks.toml around lines 6 - 35, Anchor the Gitleaks allowlist path
regexes so each suppressions only matches the exact intended test file instead
of any longer path containing the filename fragment. Update the allowlist
entries in .gitleaks.toml by tightening the paths used for the protocolDetector
and prepareEvaosDesktopBridgeResource fixtures, keeping the regexes scoped to
those specific test files. Apply the same anchoring pattern consistently across
all affected allowlist entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/public-security-scan.yml:
- Around line 36-40: Disable persisted Git credentials in the scan workflow
checkouts by updating the existing actions/checkout steps to set
persist-credentials to false. Apply this to both scan jobs in the
public-security-scan workflow so later steps cannot reuse the checkout token,
and use the existing Checkout step configuration as the place to make the
change.

In `@mobile/scripts/build.js`:
- Around line 119-123: Normalize the Apple env handling in build.js so
submission accepts the same names used by CI/docs. Update the submit flow around
the EXPO_APPLE_ID/EXPO_APPLE_TEAM_ID checks in the relevant submit helper(s) and
the direct submit path so they fall back to APPLE_ID/appleId and TEAM_ID/teamId
instead of exiting early when only the legacy names are present. Keep the logic
centered on the existing submit-related symbols and preserve the current EXPO_*
precedence.

In `@scripts/security/check-public-sensitive-content.js`:
- Around line 155-250: The exported scanner API lacks required JSDoc, so add
concise documentation for scanPath, scanText, and scanRepository describing
their parameters, return values, and any notable behavior. Place the comments
directly above each function definition in check-public-sensitive-content.js,
using the existing symbols scanPath, scanText, and scanRepository so the public
module.exports API is properly documented.
- Around line 95-129: The public security scanner is flagging its own fixture
secrets because the allowlist in ALLOWLIST does not include
scripts/security/check-public-sensitive-content.js. Update isAllowed() support
by adding this file to the allowlist with the existing fake-secret
ruleIds/patterns used in ALLOWLIST so the scanner recognizes its
self-referential test data and does not fail CI when scanning this script.

---

Outside diff comments:
In @.gitleaks.toml:
- Around line 6-35: Anchor the Gitleaks allowlist path regexes so each
suppressions only matches the exact intended test file instead of any longer
path containing the filename fragment. Update the allowlist entries in
.gitleaks.toml by tightening the paths used for the protocolDetector and
prepareEvaosDesktopBridgeResource fixtures, keeping the regexes scoped to those
specific test files. Apply the same anchoring pattern consistently across all
affected allowlist entries.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c5d47b8b-ae2d-4df0-8feb-40b610f8284c

📥 Commits

Reviewing files that changed from the base of the PR and between b91c11b and b365019.

📒 Files selected for processing (8)
  • .github/workflows/pr-checks.yml
  • .github/workflows/public-security-scan.yml
  • .gitleaks.toml
  • mobile/eas.json
  • mobile/scripts/build.js
  • package.json
  • scripts/security/check-public-sensitive-content.js
  • tests/unit/process/checkPublicSensitiveContent.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Thin App Smoke (macos-arm64)
🧰 Additional context used
📓 Path-based instructions (4)
**/*[!.module].ts

📄 CodeRabbit inference engine (AGENTS.md)

Utility files must use camelCase naming (e.g., formatDate.ts)

Files:

  • tests/unit/process/checkPublicSensitiveContent.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Unused function parameters must be prefixed with underscore (_)
Use single-element arrays inline if they fit on one line; require trailing commas in multi-line arrays/objects; use single quotes for strings
Use English for code comments; provide JSDoc for public functions

Files:

  • tests/unit/process/checkPublicSensitiveContent.test.ts
  • scripts/security/check-public-sensitive-content.js
  • mobile/scripts/build.js
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use @icon-park/react for icons
TypeScript strict mode must be enabled; no 'any' types, no implicit returns
Use path aliases: @/, @process/, @renderer/* for imports
Prefer 'type' over 'interface' for type definitions (per Oxlint config)

Files:

  • tests/unit/process/checkPublicSensitiveContent.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Vitest 4 for testing with a coverage target of at least 80%

Files:

  • tests/unit/process/checkPublicSensitiveContent.test.ts
🪛 ast-grep (0.44.0)
scripts/security/check-public-sensitive-content.js

[warning] 7-7: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: require('node:child_process')
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process)


[warning] 19-19: Detects non-literal values in regular expressions
Context: new RegExp((?:^\\s*|[{,]\\s*)["']?(?:${keyPattern})["']?\\s*:\\s*["']([^"']+)["'], 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).

(detect-non-literal-regexp)


[warning] 21-21: Detects non-literal values in regular expressions
Context: new RegExp((?:^|\\s)(?:${keyPattern})\\s*=\\s*["']?([^"'\\s]+)["']?, 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).

(detect-non-literal-regexp)

🪛 GitHub Actions: Public Security Scan / 0_Public Sensitive Content.txt
scripts/security/check-public-sensitive-content.js

[error] 99-99: Public sensitive content check failed: openai-api-key Provider API keys must never be committed. [REDACTED:openai-api-key]


[error] 110-110: Public sensitive content check failed: test-fixture-secret Secret-looking test fixtures must stay in approved test files only. [REDACTED:test-fixture-secret]


[error] 111-111: Public sensitive content check failed: test-fixture-secret Secret-looking test fixtures must stay in approved test files only. [REDACTED:test-fixture-secret]


[error] 118-118: Public sensitive content check failed: test-fixture-secret Secret-looking test fixtures must stay in approved test files only. [REDACTED:test-fixture-secret]

🪛 GitHub Actions: Public Security Scan / Public Sensitive Content
scripts/security/check-public-sensitive-content.js

[error] 99-99: Public sensitive content check failed: Provider API keys must never be committed. (Detected: [REDACTED:openai-api-key])


[error] 110-110: Public sensitive content check failed: Secret-looking test fixtures must stay in approved test files only. (Detected: [REDACTED:test-fixture-secret])


[error] 111-111: Public sensitive content check failed: Secret-looking test fixtures must stay in approved test files only. (Detected: [REDACTED:test-fixture-secret])


[error] 118-118: Public sensitive content check failed: Secret-looking test fixtures must stay in approved test files only. (Detected: [REDACTED:test-fixture-secret])

🪛 zizmor (1.26.1)
.github/workflows/public-security-scan.yml

[warning] 19-22: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[warning] 36-39: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[warning] 52-55: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[warning] 12-12: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment

(undocumented-permissions)


[warning] 3-8: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting

(concurrency-limits)

🔇 Additional comments (3)
mobile/eas.json (1)

30-41: LGTM!

mobile/scripts/build.js (2)

16-17: LGTM!

Also applies to: 33-41, 67-73, 126-130, 177-179


75-77: 🎯 Functional Correctness

No change needed for --auto-submit
mobile/scripts/build.js already requires --profile, and the --auto-submit scripts in mobile/package.json all pass one.

			> Likely an incorrect or invalid review comment.

Comment thread .github/workflows/public-security-scan.yml
Comment thread mobile/scripts/build.js Outdated
Comment thread scripts/security/check-public-sensitive-content.js
Comment thread scripts/security/check-public-sensitive-content.js
@100yenadmin
100yenadmin dismissed coderabbitai[bot]’s stale review July 1, 2026 09:40

Dismissed as stale: review targeted b365019; current head e3f6e3b addresses valid feedback, Public Security Scan and PR Checks are green, and CodeRabbit status context is success.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3f6e3b8e4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread mobile/scripts/build.js
Comment thread scripts/security/check-public-sensitive-content.js
Comment thread scripts/security/check-public-sensitive-content.js Outdated
Comment thread scripts/security/check-public-sensitive-content.js Outdated
Comment thread .github/workflows/pr-checks.yml Outdated
@100yenadmin
100yenadmin merged commit 2fbde19 into main Jul 1, 2026
13 checks passed
@100yenadmin
100yenadmin deleted the codex/public-security-audit-20260630 branch July 1, 2026 10:11

@evaos-code-review-bot evaos-code-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Walkthrough

PR: #497 - [codex] Guard public repo sensitive content
Head: 102fb96d58b3bfc6b5ec82b6bddba5e2c09db5a4 into main. Review event: REQUEST_CHANGES.

Estimated review effort: 5/5 (~66 min)

Changed Files

File Status Churn Purpose Risk
.github/workflows/pr-checks.yml modified +24/-2 Changed file Low
.github/workflows/public-security-scan.yml added +87/-0 Changed file Moderate: validated P2 finding
.gitleaks.toml added +34/-0 Changed file Elevated: validated P1 finding
mobile/eas.json modified +8/-4 Changed file Low
mobile/scripts/build.js modified +56/-14 Changed file Moderate: validated P3 finding
package.json modified +1/-0 Configuration Low
scripts/security/check-public-sensitive-content.js added +296/-0 Changed file Moderate: validated P2 finding
tests/unit/process/checkPublicSensitiveContent.test.ts added +113/-0 Test coverage Low

Review Signal

Validated inline findings: 4 (P0: 0, P1: 1, P2: 2, P3: 1).
Dropped findings before posting: 1. High-severity findings: 1.

Related Context

Related issues/PRs: #496.
Suggested labels: bug, tests.
Suggested reviewers: none from current metadata.

Pre-merge checklist

  • Inline comments target current RIGHT-side diff lines.
  • No secret-like content survived into posted inline comments.
  • REQUEST_CHANGES is only used when P0/P1 findings survive validation.
  • Labels and reviewers are suggestions only; the bot did not auto-apply them.

Comment thread .gitleaks.toml
description = "Fake Google API key protocol detector fixture"
condition = "AND"
paths = ['''^tests/unit/common/protocolDetector\.test\.ts$''']
regexes = ['''AIzaa{35}''']

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Gitleaks allowlist regex AIzaa{35} over-matches; will not match the 35-a fixture and may leak/err

In the 'Fake Google API key protocol detector fixture' allowlist, regexes = ['''AIzaa{35}'''] is interpreted by Gitleaks as a Go regexp: AIza + one literal a + the quantifier {35}, i.e. it matches AIza followed by 36 a's. The test fixture it is meant to allow (tests/unit/common/protocolDetector.test.ts) builds the key as 'AIza' + 'a'.repeat(35) = AIza + 35 a's (39 chars total). The allowlist therefore does NOT match the fixture it claims to allow. Either the Gitleaks job will flag the legitimate test fixture (CI-breaking false positive) or, worse, the allowlist silently matches nothing and the rule provides no real coverage. The in-repo custom scanner uses a correctly escaped pattern (/'''AIzaa\{35\}'''/, line 118 of check-public-sensitive-content.js), confirming intent was a literal. Fix: align .gitleaks.toml with the literal the fixture produces, e.g. regexes = ['''AIzaaaa{34}'''] won't help; use the explicit literal AIza + 35 a's, or drop the meta-quantifier.

Why this matters: A misconfigured allowlist either breaks the new required CI gate (Gitleaks Full History) with false positives on a known fixture, or silently disables coverage so a real Google key could pass review.

set -euo pipefail
TRUSTED_SCANNER="trusted-security-base/scripts/security/check-public-sensitive-content.js"
if [ "${{ github.event_name }}" = "pull_request" ] && [ -f "$TRUSTED_SCANNER" ]; then
node "$TRUSTED_SCANNER" "$PWD"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Trusted scanner runs against PR checkout, so PR can neuter its own gate

In 'Run repo-owned public scan', when the trusted scanner is present it is executed as node trusted-security-base/... "$PWD". $PWD is the PR-head checkout, and scanRepository enumerates files via git ls-files -z using the PR's own .git index and working tree. The scanner binary comes from the base ref, but the file set and content it inspects are entirely attacker/PR-controlled. A PR that adds, say, a tracked secrets/.env or a real sk-... key to a non-allowlisted path is scanned — good — but a PR can also ADD allowlist entries or change which files are enumerated because it controls the working tree and index. Since the scanner source is from base, allowlist edits in the PR won't take effect (good), but the file enumeration and content are still PR-controlled, so any rule the PR can evade by file placement (e.g., a >8MB file, or a binary that isProbablyTextBuffer skips) bypasses the gate. This is acceptable for a public gate but should be documented; consider scanning git diff of the PR rather than the full tracked set, or at minimum note that >MAX_TEXT_FILE_BYTES and NUL-containing files are skipped silently (no finding emitted), which is a real coverage gap for a secrets gate.

Why this matters: A public secrets gate that silently skips large or binary files can miss committed secrets; the threat model should at least be explicit so reviewers know the gate is best-effort, not complete.

continue;
}
const stats = statSync(absolutePath);
if (!stats.isFile() || stats.size > MAX_TEXT_FILE_BYTES) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Secrets gate silently skips files >8MB and binary; no warning emitted

scanRepository skips any file larger than MAX_TEXT_FILE_BYTES (8 MiB) or whose buffer contains a NUL byte (isProbablyTextBuffer), continuing with no finding and no log line. For a public-repository secrets gate this is a meaningful blind spot: a leaked .log or large generated file over 8 MiB, or a secret embedded in a file that happens to contain a single NUL byte, will pass silently. Unlike the risky-path rule, these skips produce no audit signal. Consider emitting a ::warning:: (or a low-severity finding) for skipped tracked files above the size/text threshold so the gate is transparent about what it did not inspect.

Why this matters: Silent skips in a secrets scanner create a false sense of coverage; operators may trust the green check while real secrets in large/binary tracked files go uninspected.

Comment thread mobile/scripts/build.js
const localOutputPath = path.join(projectRoot, `build-${Date.now()}${outputExt}`);
let buildArgs = args.filter((a) => a !== '--auto-submit' && a !== '--direct-submit');

if (!isLocal && autoSubmit && !buildArgs.includes('--auto-submit-with-profile')) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Non-local auto-submit rewrites --auto-submit to --auto-submit-with-profile but eas submit profiles no longer carry Apple credentials

For cloud (!isLocal) builds with --auto-submit, the script strips --auto-submit and pushes --auto-submit-with-profile <profile>, then relies on EXPO_APPLE_ID/EXPO_APPLE_TEAM_ID being set in appleEnv (validated above). That part is consistent. However, the EAS submit profiles in mobile/eas.json (production/preview) were stripped of appleId/appleTeamId in this same PR, so EAS submit now depends entirely on these env vars being present at submit time. The validation at lines 50-56 covers iOS, but if a caller runs --auto-submit with --platform android (or no platform defaulting to ios) the env checks are skipped for android — which is correct, but there is no assertion that the submit profile actually exists for the platform. This is low risk (EAS will error clearly) but the coupling between the removed eas.json fields and the new env-var contract is undocumented; a maintainer running cloud iOS auto-submit without the env vars set will get a failure inside EAS rather than the script's clear requireEnv message, because requireEnv only runs for autoSubmit when !expoAppleTeamId — and a partial set (id present, team id missing) is handled, but the reverse ordering is fine. Net: behavior is correct, but consider adding a one-line comment in eas.json or build.js documenting that Apple credentials are now env-only.

Why this matters: The implicit contract between eas.json (credentials removed) and build.js (env vars now mandatory) is easy to break in a future edit; a comment prevents regression.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 102fb96d58

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +62 to +63
- name: Run Gitleaks
uses: gitleaks/gitleaks-action@e0c47f4f8be36e29cdc102c57e68cb5cbf0e8d1e

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Run Gitleaks with trusted PR configuration

On pull_request, this checkout is the PR merge tree and the Gitleaks action is invoked without GITLEAKS_CONFIG; the action docs say a root gitleaks.toml is automatically detected and used, so a PR can commit a broad allowlist or disabled rules alongside a real npm/Stripe/database secret and make this new security job pass. Mirror the trusted-base pattern used for the repo-owned scanner, or otherwise point Gitleaks at a base-branch config while scanning the PR tree.

Useful? React with 👍 / 👎.

value.startsWith('${') ||
value.startsWith('$') ||
value.startsWith('process.env.') ||
/^(?:<|YOUR_|REPLACE_|example|test|fake|dummy)/i.test(value)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop exempting Apple IDs by broad prefixes

Because this placeholder check returns before applying the Apple email/team regex, any hardcoded value that merely starts with test, fake, dummy, or example is ignored; for example, EXPO_APPLE_ID=testflight@company.com or a real team ID beginning with TEST would pass the required public scan even though the rule is meant to block hardcoded Apple identifiers. Restrict the exemption to exact placeholder values/domains instead of broad prefixes.

Useful? React with 👍 / 👎.

BUNDLE_ID="$(/usr/libexec/PlistBuddy -c 'Print :CFBundleIdentifier' "$APP_PATH/Contents/Info.plist")"
VERSION="$(/usr/libexec/PlistBuddy -c 'Print :CFBundleShortVersionString' "$APP_PATH/Contents/Info.plist")"
BRIDGE_SUMMARY="$(node -e "const fs=require('fs'); const m=JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); if (m.placeholder !== true) { console.error('::error::Expected thin-app-smoke to bundle the diagnostic Bridge placeholder'); process.exit(1); } console.log('diagnostic-placeholder')" "$BRIDGE_MANIFEST")"
BRIDGE_SUMMARY="$(node -e "const fs=require('fs'); const m=JSON.parse(fs.readFileSync(process.argv[1], 'utf8')); const sha=/^[0-9a-f]{40}$/i; if (m.placeholder === true) { console.log('diagnostic-placeholder'); process.exit(0); } if (!sha.test(String(m.requestedSourceRef || '')) || !sha.test(String(m.sourceCommit || '')) || m.requestedSourceRef !== m.sourceCommit) { console.error('::error::Expected thin-app-smoke to bundle a diagnostic Bridge placeholder or the pinned real Bridge source'); process.exit(1); } console.log('pinned-real-bridge@' + String(m.sourceCommit).slice(0, 12));" "$BRIDGE_MANIFEST")"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Compare Bridge manifest to the pinned workflow ref

In the thin-app smoke workflow, the build step pins EVAOS_DESKTOP_BRIDGE_SOURCE_REF to a specific commit, but this verification only checks that the manifest's requestedSourceRef and sourceCommit agree with each other. If the packaging code stops honoring the workflow pin and packages a different Bridge commit while writing matching manifest fields, this new acceptance path still passes; compare sourceCommit against the pinned workflow ref instead of trusting the manifest's requested ref.

Useful? React with 👍 / 👎.

function hasUnsafeLiteralAssignment(line, keys, valuePattern) {
const keyPattern = keys.map(escapeRegExp).join('|');
const jsonLikeAssignment = line.match(
new RegExp(`(?:^\\s*|[{,]\\s*)["']?(?:${keyPattern})["']?\\s*:\\s*["']([^"']+)["']`, 'i')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Scan unquoted YAML Apple identifiers

This colon-assignment branch only inspects quoted values, so a normal YAML environment block such as EXPO_APPLE_ID: developer@company.com or EXPO_APPLE_TEAM_ID: AB12CD34EF in a workflow is not reported by the required public scan. Since workflow/env YAML commonly omits quotes, include unquoted colon values before applying the Apple email/team regex.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

evaos evaOS public beta R&D work kind:integration Integration implementation issue risk:security Security, auth, secrets, permission risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Audit public repo for secrets, PII, and security guardrails

1 participant