Add fuzzing for wta#17
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
tools/wta/src/shell_fuzz.rs:29
- The quoting/escaping here is not compatible with the Windows command-line parsing rules used by
CreateProcess/CommandLineToArgvW: doubling embedded"(producing"") does not reliably round-trip, and backslash cases (e.g. trailing\, or\"sequences) are not handled. Please switch to a Windows-style quoting algorithm that escapes quotes by prefixing backslashes and correctly handles runs of backslashes (seeQuoteAndEscapeCommandlineArginsrc/cascadia/WinRTUtils/inc/WtExeUtils.h:123-147for an existing implementation to mirror).
// Quote args containing spaces or double quotes
if arg.contains(' ') || arg.contains('"') {
cmdline.push('"');
// Escape embedded double quotes by doubling them
for ch in arg.chars() {
if ch == '"' {
cmdline.push('"');
}
cmdline.push(ch);
}
cmdline.push('"');
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
tools/wta/src/shell/shell_manager.rs:13
- Using
#[path = "../shell_fuzz.rs"] mod shell_fuzz;compiles the same source twice (once in the lib target and once in the bin target) and relies on a relative path from a submodule. Now that the package has a properlib.rsre-exportingbuild_wt_commandline, the binary can import it from thewtalibrary crate instead, avoiding duplication and making the sharing mechanism more robust.
/// Configuration for creating a new terminal.
pub struct TerminalConfig {
pub command: String,
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
PankajBhojwani
force-pushed
the
pabhojwa/intelligent/wta_fuzzing
branch
from
d1218fa to
14760ed
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report
|
| Dictionary | Entries | Covers | Uniquely |
|---|---|---|---|
| cspell:csharp/csharp.txt | 32 | 1 | 1 |
| cspell:aws/aws.txt | 232 | 1 | 1 |
| cspell:fonts/fonts.txt | 536 | 1 | 1 |
Consider adding to the extra_dictionaries array (in the .github/actions/spelling/config.json file):
"cspell:csharp/csharp.txt",
"cspell:aws/aws.txt",
"cspell:fonts/fonts.txt",
To stop checking additional dictionaries, put (in the .github/actions/spelling/config.json file):
"check_extra_dictionaries": []Forbidden patterns 🙅 (14)
In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves.
These forbidden patterns matched content:
Homoglyph (Turkish) should be i
[ı](?=[A-Za-z]{2,})|(?<=[A-Za-z]{2})[ı]|(?<=[A-Za-z])[ı](?=[A-Za-z])
Should be nonexistent
\b[Nn]o[nt][- ]existent\b
Should be preexisting
[Pp]re[- ]existing
Should be a
\san (?=(?:[b-dgjkpqtvwz]|f(?!f|d\b)|h(?!onest|onou?r|our|s[lv]|tml|ttp|ref)|l(?!cov)|n(?!ginx|grok|pm)|r(?!c)|s(?!s[ho]|log|vg))[a-z]|x(?!\b|[-\d]|ml))
Should be an
(?<!(?:\b[Ii]|git) )(?<![-.])(?<!\d\s?)\bam\b(?!/pm|[:")]| I\b)
Should be ; otherwise or . Otherwise
https://study.com/learn/lesson/otherwise-in-a-sentence.html
, [Oo]therwise\b
Should probably be Otherwise,
(?<=\. )Otherwise\s
Complete sentences in parentheticals should not have a space before the period.
\s\.\)(?!.*\}\})
Should be an
(?<=\s)a(?= (?:a(?!nd\s|s\s)|e(?!u)|i(?![ns]\s)|o(?!f\b|nc?e)|u(?!\d|biquitous|int|kr|n[ai]|r[ael]|s[aeiu]|short|tf\d*|t_|til|topia|uid|vula|v\b)|y(?!aml|arn|e|ie|oga|oung|y)))
Should be set up (setup is a noun / set up is a verb)
\b[Ss]etup(?= (?:an?|the|to)\b)
Should be reentrancy
[Rr]e[- ]entrancy
Should be reentrant
[Rr]e[- ]entrant
Should be whether or not ...
(?i)\b(?:whe|ra)ther(?:\s\w+)+ or not\.
Should be WinGet
\bWinget\b
Pattern suggestions ✂️ (1)
You could add these patterns to .github/actions/spelling/patterns/a65eb465d66737678eb336a2625928d78fd92e83.txt:
# Automatically suggested patterns
# hit-count: 1 file-count: 1
# python
\b(?i)py(?!gment|gmy|lon|ramid|ro|th)(?=[a-z]{2,})
Alternatively, if a pattern suggestion doesn't make sense for this project, add a # to the beginning of the line in the candidates file with the pattern to stop suggesting it.
Errors, Warnings, and Notices ❌ (8)
See the 📂 files view, the 📜action log, 👼 SARIF report, or 📝 job summary for details.
See ❌ Event descriptions for more information.
✏️ Contributor please read this
By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
- ... misspelled, then please correct them instead of using the command.
- ... names, please add them to
.github/actions/spelling/allow/names.txt. - ... APIs, you can add them to a file in
.github/actions/spelling/allow/. - ... just things you're using, please add them to an appropriate file in
.github/actions/spelling/expect/. - ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in
.github/actions/spelling/patterns/.
See the README.md in each directory for more information.
🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉
If the flagged items are 🤯 false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txtfile matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^refers to the file's path from the root of the repository, so^README\.md$would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it,
try adding it to thepatterns.txtfile.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
|
Thanks Pankaj for the PR. Fuzzing adds CI cost without proportional value here — the input is internal-only (wt => wta, not user-facing) I will create a seperate PR even:
|
The input is coming from the LLM and then getting injected into wt (via the commandline builder). I don't think we can assume that what the LLM produces is "safe" or internal, hence the fuzzing. |
r4-human-only verifier caught a real semantic bug: the previous "any comment by us in the last 100" semantic miscounted threads where Copilot **re-raises** after our reply — last=copilot but the script saw our older reply and reported "not awaiting" even though the ball was clearly back in our court. Fix: count threads where the LAST comment is NOT from the authenticated user. This is the "ball-in-court" model: - Copilot/human posts a finding → last=them → awaiting our reply. - We reply → last=us → ball passes back → not awaiting. - Copilot re-raises after our reply → last=them again → awaiting. Empirically verified on yeelam-gordon/ABCSkillTest: - PR #17 baseline 38/38, after 1 reply-NoResolve r3 showed 38/37 earlier; current measurement is 37/36 (we resolved 1 thread since, matching the math exactly). - PR #18 (human-only fresh) Mode=human-only correctly reported. awaiting=0 in that single-account test setup because the agent's gh user is also the human reviewer — an unavoidable identity- conflation limitation of the test, NOT a script bug. In real production usage the agent's gh user is distinct from the reviewer's identity (Copilot bot OR a separate human). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary of the Pull Request
fixed a quoting bug: embedded " in args was not escaped)
submission
Validation Steps Performed
wtastill builds normallyPR Checklist