Conversation
…ains arguments Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
There was a problem hiding this comment.
Pull request overview
Updates AWF command rendering to use double-quoted --allow-domains/--block-domains argument values so generated workflow run: scripts are ShellCheck SC1003-clean (notably for wildcard domains like *.githubusercontent.com), and recompiles affected golden + lock outputs accordingly.
Changes:
- Adjust
BuildAWFArgsto pre-wrap allow/block domain lists in double quotes soshellEscapeArgpreserves them. - Update WASM golden fixtures to expect double-quoted domain lists.
- Recompile GitHub Actions workflow lock files to remove single-quoted domain lists.
Reviewed changes
Copilot reviewed 162 out of 162 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/workflow/awf_helpers.go | Pre-wraps --allow-domains / --block-domains values in double quotes when building AWF args. |
| pkg/workflow/testdata/wasm_golden/TestWasmGolden_CompileFixtures/basic-copilot.golden | Updates expected compiled output to include double-quoted --allow-domains list. |
| pkg/workflow/testdata/wasm_golden/TestWasmGolden_CompileFixtures/with-imports.golden | Updates expected compiled output to include double-quoted --allow-domains list. |
| .github/workflows/*.lock.yml (many files) | Recompiled lock workflows reflecting the updated quoting for --allow-domains. |
Comments suppressed due to low confidence (1)
pkg/workflow/awf_helpers.go:163
- Same concern for
blockedDomains: unlikenetwork.allowed, blocked domains are commonly user-specified and may not be run through the same domain-pattern validation. Pre-quoting with double quotes means values like$(...)or$VARwould be expanded by the shell in compiled workflows. Please either validate blocked domains with the same strict domain-pattern rules (rejecting$/backticks/etc.) or use a safe double-quoting helper that escapes expansion-sensitive characters instead of bypassingshellEscapeArg.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/workflow/awf_helpers.go
Outdated
| // Pre-wrap in double quotes so shellEscapeArg preserves them (wildcards like *.domain.com | ||
| // are command arguments, not shell globs, and double quotes prevent SC1003 ShellCheck warnings) | ||
| awfArgs = append(awfArgs, "--allow-domains", "\""+config.AllowedDomains+"\"") |
There was a problem hiding this comment.
Wrapping the domain list in double quotes here bypasses shellEscapeArg’s escaping logic (it short-circuits on pre-quoted strings). If AllowedDomains ever contains $, backticks, or backslashes, double quotes will allow shell expansion/command substitution in the generated run: script. Consider keeping AllowedDomains unquoted and instead teaching shellEscapeArg (or a dedicated helper used here) to emit a safely-escaped double-quoted form for domain lists (escaping at least \, ", $, and `) so you fix SC1003 without weakening shell-safety.
This issue also appears on line 161 of the same file.
There was a problem hiding this comment.
Good catch. Added a shellDoubleQuoteArg helper to shell.go (commit 95170df) that escapes \, ", $, and ` before wrapping in double quotes — the same escape sequence used by shellEscapeCommandString. awf_helpers.go now uses this helper instead of naive string concatenation, so values containing those characters can't trigger shell expansion.
|
Smoke test results:
|
|
Smoke Test Results for run §22303636050
Overall: PASS ✅ —
|
There was a problem hiding this comment.
This PR applies a systematic fix for SC1003 shellcheck warnings by wrapping --allow-domains argument values in double quotes across all lock files. The change is correct and consistent — it ensures shell word-splitting doesn't occur on domain lists that contain wildcard characters like *. No logic changes, purely a quoting fix.
📰 BREAKING: Report filed by Smoke Copilot
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
Commit pushed: |
|
Commit pushed: |
|
Smoke Test Results — Run §22303636093 — Overall: PARTIAL Core (#1–10): ✅✅✅✅❌✅✅✅✅✅ ❌ Test 5 (Playwright MCP):
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude
|
@copilot fix playwright arguments |
…ains, remove --browser-arg Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
Commit pushed: |
|
🤖 Smoke test §22304444079 results for
Overall: ✅ PASS
|
There was a problem hiding this comment.
This PR correctly fixes SC1003 shellcheck warnings by wrapping --allow-domains and --block-domains argument values in double quotes across all affected workflow lock files. The change is minimal and targeted. No issues found.
📰 BREAKING: Report filed by Smoke Copilot
| --- | ||
| "gh-aw": patch | ||
| --- | ||
| Double-quote `awf --allow-domains`/`--block-domains` arguments so SC1003 warnings disappear while keeping wildcard lists intact. |
There was a problem hiding this comment.
Good changeset description — clearly explains the SC1003 fix. The double-quoting of domain lists is the right approach for shell compatibility.
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
PR titles: [docs] Update glossary - weekly full scan; [specs] Update layout specification - 2026-02-23
|
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude
ShellCheck SC1003 was firing 156 times across compiled workflow lock files because the compiler wrapped domain lists containing wildcards (e.g.
*.githubusercontent.com) in single quotes. Double quotes suppress glob expansion equally well and are SC1003-clean.Changes
pkg/workflow/shell.go— AddedshellDoubleQuoteArghelper that safely escapes\,",$, and`before wrapping in double quotes, preventing unintended shell expansion while fixing SC1003.pkg/workflow/awf_helpers.go— UseshellDoubleQuoteArgfor--allow-domainsand--block-domainsvalues instead of naive string concatenation.pkg/workflow/mcp_config_playwright_renderer.go— Replaced--browser-arg --no-sandboxwith the direct--no-sandboxflag, which is the correct interface in@playwright/mcpv0.0.26+.pkg/workflow/testdata/wasm_golden/.../*.golden— Updated 3 golden files to expect double-quoted domain lists and updated playwright entrypoint args..github/workflows/*.lock.yml— All 158 lock files recompiled; zero single-quoted domain lists remain and--browser-argis removed from all playwright configurations.Before →
--allow-domains '*.githubusercontent.com,api.github.com,...'After →
--allow-domains "*.githubusercontent.com,api.github.com,..."Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.
Changeset
Warning
The following domains were blocked by the firewall during workflow execution:
codeload.github.comgithub.com✨ PR Review Safe Output Test - Run 22303636093
Changeset
--no-sandboxflag.Warning
The following domain was blocked by the firewall during workflow execution:
github.com✨ PR Review Safe Output Test - Run 22304444078