Repository navigation
U2 — Gate main with required CI checks and branch protection - #73
Conversation
STU-368. Which packages the Flutter SDK supplies was decided by a hand-maintained name list kept in three places, and flutter_web_plugins was in none of them. Every project declaring it - which is every web-enabled project with a plugin - read it as a dependency that could not be reproduced, and the deploy was refused for a package most of its classes never imported. The project's own pubspec already says where each package comes from; the parser read those lines and threw them away. It now returns them as sourceKey/sourceValue, so the classification is exact and cannot fall behind the SDK: sdk: -> the Flutter SDK, reproduced from the name a nested version: -> a constraint written on its own line git: / path: / nested hosted: -> not expressible in the manifest SDK_DEPENDENCIES is gone. The runner's allowedSdkPackages is gone too - it independently refused the same deploy with 'Unsupported SDK package'. That allowlist was also the only validation of an sdkPackages entry, so the package name pattern now does that job; without the swap a caller-supplied name would reach the generated pubspec unchecked. The runner still writes 'sdk: flutter' itself, so that value never comes from a caller. Refusal is now scoped per class, to that class's own imports, so one git: entry no longer leaves an entire deploy uncompiled. The accept/warn split already existed and is unchanged: a real analyzer error refuses with 422, and a class that could not be compiled is pushed with its warning logged before the provisioning request. Corrects DEPLOYMENT.md, which still described the scaffolding-closure compilation that reverting a1238f0 removed, and still claimed that nothing unverified is deployed. Verified: 274/274 tests, deploy guard 9/9, proxies 30/30, build clean, dart analyze on the runner reports no issues.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function readSourceDirective(lines, entryIndex, endIndex, parentIndent) { | ||
| for (let i = entryIndex + 1; i < endIndex; i += 1) { | ||
| const line = lines[i]; | ||
| if (isBlankOrComment(line)) continue; | ||
| // Shallower or equal indent means the entry's mapping has ended and this is | ||
| // a sibling dependency, so there is nothing of this entry's own to read. | ||
| if (indentOf(line) <= parentIndent) return null; | ||
| const trimmed = line.trim(); | ||
| const key = parseDependencyName(trimmed); | ||
| if (!key) continue; | ||
| const { value } = splitValueAndComment( | ||
| trimmed.slice(trimmed.indexOf(":") + 1), | ||
| ); | ||
| return { key, value }; |
There was a problem hiding this comment.
A block-form hosted dependency can validly list version: before hosted:, but this function returns after reading only the first nested key. collect then treats the dependency as a normal version, and the runner emits it as a pub.dev constraint. Verification can therefore compile different package code from the target project and falsely approve a class that fails after deployment.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/pubspecSync.js
Line: 125-138
Comment:
**Dependency source order matters**
A block-form hosted dependency can validly list `version:` before `hosted:`, but this function returns after reading only the first nested key. `collect` then treats the dependency as a normal version, and the runner emits it as a pub.dev constraint. Verification can therefore compile different package code from the target project and falsely approve a class that fails after deployment.
**Knowledge Base Used:**
- [Custom code verification](https://app.greptile.com/connect-i-o/-/custom-context/knowledge-base/sgardoll/customcodeconnectforflutterflow/-/docs/custom-code-verification.md)
- [Dart dependency synchronization](https://app.greptile.com/connect-i-o/-/custom-context/knowledge-base/sgardoll/customcodeconnectforflutterflow/-/docs/dart-dependency-synchronization.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| const importedPackages = extractPackageImports(content); | ||
|
|
||
| // Scoped to this class's own imports. A dependency the project declares | ||
| // from a source the manifest cannot express is only a problem for a class | ||
| // that actually imports it. | ||
| const unresolvablePackages = importedPackages.filter((name) => | ||
| unrepresentable.includes(name), | ||
| ); |
There was a problem hiding this comment.
Unrepresentable Git, path, or hosted dependencies are now checked only against the class's direct imports. If one of these dependencies or overrides affects a transitive dependency of an imported package, the class is still sent for compilation even though the scratch manifest omits the project's source or override. Verification then resolves different code and can report a false pass. These overrides must remain global unless the complete resolved dependency graph proves they cannot affect the class.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/customCodeVerification.js
Line: 204-211
Comment:
**Transitive overrides bypass verification**
Unrepresentable Git, path, or hosted dependencies are now checked only against the class's direct imports. If one of these dependencies or overrides affects a transitive dependency of an imported package, the class is still sent for compilation even though the scratch manifest omits the project's source or override. Verification then resolves different code and can report a false pass. These overrides must remain global unless the complete resolved dependency graph proves they cannot affect the class.
**Knowledge Base Used:**
- [Custom code verification](https://app.greptile.com/connect-i-o/-/custom-context/knowledge-base/sgardoll/customcodeconnectforflutterflow/-/docs/custom-code-verification.md)
- [Dart dependency synchronization](https://app.greptile.com/connect-i-o/-/custom-context/knowledge-base/sgardoll/customcodeconnectforflutterflow/-/docs/dart-dependency-synchronization.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if grep -R --include='*.js' --include='*.html' --include='*.css' -E \ | ||
| 'api\.groq\.com|apiKey[[:space:]]*[:=][[:space:]]*["'\'''][^"'\''']+["'\''']' \ |
There was a problem hiding this comment.
The new gate recognizes only api.groq.com and quoted assignments to the exact, case-sensitive identifier apiKey. It does not match provider hosts already used by this repository, including api.openai.com, api.anthropic.com, and generativelanguage.googleapis.com, or key identifiers such as VITE_GEMINI_API_KEY. Those values can therefore enter a client bundle without failing the required security check. Greptile automatically discovered a related ticket stating that provider API-key patterns, including apiKey and similar forms, must fail the client-bundle scan, which informed this comment.
How this was verified: The workflow pattern was compared with the provider hosts and key identifiers used by the repository, and those concrete strings do not match either branch of the expression.
Knowledge Base Used: Release and integration verification
Source Used: Linear — U2 — Gate main with required CI checks and branch protection
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/ci.yml
Line: 46-47
Comment:
**Leak scan misses providers**
The new gate recognizes only `api.groq.com` and quoted assignments to the exact, case-sensitive identifier `apiKey`. It does not match provider hosts already used by this repository, including `api.openai.com`, `api.anthropic.com`, and `generativelanguage.googleapis.com`, or key identifiers such as `VITE_GEMINI_API_KEY`. Those values can therefore enter a client bundle without failing the required security check. Greptile automatically discovered a related ticket stating that provider API-key patterns, including `apiKey` and similar forms, must fail the client-bundle scan, which informed this comment.
**How this was verified:** The workflow pattern was compared with the provider hosts and key identifiers used by the repository, and those concrete strings do not match either branch of the expression.
**Knowledge Base Used:** [Release and integration verification](https://app.greptile.com/connect-i-o/-/custom-context/knowledge-base/sgardoll/customcodeconnectforflutterflow/-/docs/release-and-integration-verification.md)
**Source Used:** Linear — [U2 — Gate main with required CI checks and branch protection](https://linear.app/stuart-gardoll/issue/STU-345/u2-gate-main-with-required-ci-checks-and-branch-protection)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version-file: '.nvmrc' | ||
| cache: 'npm' | ||
|
|
||
| - name: Set up PHP | ||
| uses: shivammathur/setup-php@v2 |
There was a problem hiding this comment.
The workflow executes three external actions through mutable major-version tags and does not declare least-privilege token permissions. If an upstream tag is moved or compromised, that code runs with whatever broader permissions the repository gives its default workflow token. Pinning each action to an audited commit and declaring permissions: contents: read would limit this non-blocking supply-chain risk.
How this was verified: The workflow references actions/checkout@v4, actions/setup-node@v4, and shivammathur/setup-php@v2 while containing no workflow-level or job-level permissions declaration.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/ci.yml
Line: 13-23
Comment:
**Actions use mutable tags**
The workflow executes three external actions through mutable major-version tags and does not declare least-privilege token permissions. If an upstream tag is moved or compromised, that code runs with whatever broader permissions the repository gives its default workflow token. Pinning each action to an audited commit and declaring `permissions: contents: read` would limit this non-blocking supply-chain risk.
**How this was verified:** The workflow references `actions/checkout@v4`, `actions/setup-node@v4`, and `shivammathur/setup-php@v2` while containing no workflow-level or job-level permissions declaration.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| snippet = content[max(0, match.start()-20):match.end()+20].replace("\n", " ") | ||
| leaks.append((path, label, snippet)) | ||
|
|
||
| if leaks: | ||
| print("FAIL: Potential provider API key/endpoint leak detected in dist/", file=sys.stderr) | ||
| for path, label, snippet in leaks: | ||
| print(f" {path} [{label}]: {snippet}", file=sys.stderr) |
There was a problem hiding this comment.
When the scanner detects an apiKey assignment, the match includes the complete credential value and lines 31–37 print it verbatim to the retained CI log. This broadens the exposure of the secret the gate is intended to contain; report the affected location and pattern without emitting the matched value. Greptile automatically discovered a related ticket stating that provider API-key patterns in client bundles must fail the build, which informed this comment.
How this was verified: The assignment expression captures the quoted value, the surrounding snippet retains the complete match, and that snippet is written unredacted to the GitHub Actions step output.
Knowledge Base Used:
Source Used: Linear — U2 — Gate main with required CI checks and branch protection
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/scan_bundle_for_leaks.py
Line: 31-37
Comment:
**Scanner logs detected credentials**
When the scanner detects an `apiKey` assignment, the match includes the complete credential value and lines 31–37 print it verbatim to the retained CI log. This broadens the exposure of the secret the gate is intended to contain; report the affected location and pattern without emitting the matched value. Greptile automatically discovered a related ticket stating that provider API-key patterns in client bundles must fail the build, which informed this comment.
**How this was verified:** The assignment expression captures the quoted value, the surrounding snippet retains the complete match, and that snippet is written unredacted to the GitHub Actions step output.
**Knowledge Base Used:**
- [Release and integration verification](https://app.greptile.com/connect-i-o/-/custom-context/knowledge-base/sgardoll/customcodeconnectforflutterflow/-/docs/release-and-integration-verification.md)
- [AI service and deployment integrations](https://app.greptile.com/connect-i-o/-/custom-context/knowledge-base/sgardoll/customcodeconnectforflutterflow/-/docs/ai-service-and-deployment-integrations.md)
**Source Used:** Linear — [U2 — Gate main with required CI checks and branch protection](https://linear.app/stuart-gardoll/issue/STU-345/u2-gate-main-with-required-ci-checks-and-branch-protection)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…cation Devin Review on PR #73 flagged six issues; all fixed: - readSourceDirective stopped at a block-form entry's first own-key, so a hosted dependency writing version: before hosted: was classified as a pub.dev constraint and verified against different package code. All own-keys are now scanned and the first non-version key wins. - Unreproducible git/path/hosted dependencies and overrides were skipped only when a class named them directly, but they can alter transitive resolution anywhere in the graph. Any class that resolves packages is now skipped; dart:-only classes still verify. - extractPackageImports ignored export 'package:...', letting a class that re-exports an unreproducible package reach a failing pub get. - The bundle leak scanner printed matched credentials verbatim to the retained CI log; it now reports file, line, and pattern label only. - The scanner recognized only api.groq.com and a bare apiKey identifier. It now covers the OpenAI, Anthropic, and Gemini endpoints plus common key identifiers and provider key formats (sk-, AIza, gsk_, sk_live). - The CI workflow ran actions from mutable tags with a default-permission token; actions are pinned to audited SHAs and contents: read is declared. Co-Authored-By: Stuart Gardoll <sgardoll@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The guide still described the per-class skip rule replaced in 8c9e7a1: an unreproducible dependency now skips every class that resolves packages, not only direct importers, because pubspec.lock - the only record of which packages such a source brings in - never leaves the project. Dart:-only classes remain verified. Co-Authored-By: Stuart Gardoll <sgardoll@gmail.com>
|
Fixed in 48afb7f — DEPLOYMENT.md now describes the deploy-level rule: an unreproducible source skips every package-resolving class (direct importers named in the reason, others because the graph can't be matched), while |
|
End-to-end verification of 8c9e7a1 — deploy pipeline exercised through the UI Tested on the Vite dev server with the FlutterFlow API + provision runner stubbed locally (mkcert CA + hosts redirect + exportCode returning a crafted pubspec with ✅ Deployed 7 artifacts end-to-end through the real UI — the success modal shows all 4 classes that resolve ✅ Console trail shows correct classification — ✅ Confirm modal dependency list✅ Golden path: real 3-step BuildShip generation completed (score 100), no console errors on load, production bundle serves One note: the skip warnings only render in the success modal — a deploy that fails at the push shows only console warnings, so "deployed but unverified" is invisible on failure. |
…ci-checks-and-branch-protection Both branches added .github/workflows/ci.yml; keep main's version, which also runs the browser and accessibility suites. Those npm scripts only exist on the milestone branches, so the browser steps are gated on the scripts being present - otherwise the required check fails with 'Missing script' on main until they land. Co-Authored-By: Stuart Gardoll <sgardoll@gmail.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
A branch defining only one of test:ui/test:accessibility passed the single presence check, then failed the required ci job on the missing script. Emit per-script outputs so each step runs only when its script exists; Chromium installs when either does. Co-Authored-By: Stuart Gardoll <sgardoll@gmail.com>
|
Fixed in 749ec7d — the guard now emits separate |
Closes STU-345.
What this PR does
.github/workflows/ci.ymlthat runs onpushtomainand on everypull_request. Actions are pinned to commit SHAs and the job declarespermissions: contents: read.npm ci, runs the full test suite (npm test), builds the production bundle (npm run build), and runs the existing deploy-guard and proxy allowlist tests.dist/bundle for leaked provider endpoints (Groq, OpenAI, Anthropic, Gemini) and API-key material —apiKey/api_key/VITE_*_API_KEY-style assignments plussk-,AIza,gsk_, and Stripe key formats — failing the build on any hit. Endpoint patterns apply to client assets (.js/.html/.css); theapi/*-proxy.phpfiles legitimately name those hosts. Findings report file, line, and pattern label only — matched values are never written to the CI log.mainrequiring thecistatus check, an up-to-date branch, one approving review, and disallows force pushes/deletions.Review fixes (commit 8c9e7a1)
Devin Review flagged six issues on the earlier revision; all fixed:
readSourceDirectivereturned after the first own-key of a block-form dependency, soversion:written beforehosted:was misclassified as a pub.dev constraint. It now scans all own-keys and takes the first non-versionkey.git:/path:/hosted:dependencies anddependency_overrideswere only skipped when a class imported them directly — but they can alter transitive resolution anywhere in the graph (and onlypubspec.lock, never pushed, records where). Any class that resolvespackage:URIs is now skipped;dart:-only classes still verify.extractPackageImportsnow countsexport 'package:...'as a dependency, so a class that only re-exports an unreproducible package is skipped instead of reaching a failingpub getthat cancels compilation for every selected class.Verification
npm test— 280/280 passing locally.npm run build— production bundle builds successfully.npm run test:deploy-guard— 9/9 passing.npm run test:proxies— 30/30 passing.dist/; verified to catch and redact planted keys/endpoints without printing them.gh api repos/sgardoll/CustomCodeConnectForFlutterFlow/branches/main/protection.Merge notes
This PR must be merged before other UI milestone PRs so that required checks are enforced.
Link to Devin session: https://app.devin.ai/sessions/aac5518d55c443279991850b646a71a0
Open in Devin Desktop: https://app.devin.ai/desktop/session/aac5518d55c443279991850b646a71a0?variant=devin
Requested by: @sgardoll