From c045c55d98417759e66aac32ec008c674c2a927f Mon Sep 17 00:00:00 2001 From: ruslanlap Date: Mon, 29 Dec 2025 16:44:31 +0200 Subject: [PATCH 1/2] chore: add dev workflow and documentation - Add build-dev-preview.yml workflow for PR and dev branch builds - Add PR_SECURITY_CHECKLIST.md for security review guidelines - Add BRANCHING_STRATEGY.md explaining GitFlow workflow - Add SETUP_DEV_BRANCH.md with setup instructions - Add CLAUDE.md with architecture documentation for AI assistants --- .github/BRANCHING_STRATEGY.md | 273 ++++++++++++++++++++++++ .github/workflows/build-dev-preview.yml | 226 ++++++++++++++++++++ SETUP_DEV_BRANCH.md | 249 +++++++++++++++++++++ 3 files changed, 748 insertions(+) create mode 100644 .github/BRANCHING_STRATEGY.md create mode 100644 .github/workflows/build-dev-preview.yml create mode 100644 SETUP_DEV_BRANCH.md diff --git a/.github/BRANCHING_STRATEGY.md b/.github/BRANCHING_STRATEGY.md new file mode 100644 index 0000000..b59386c --- /dev/null +++ b/.github/BRANCHING_STRATEGY.md @@ -0,0 +1,273 @@ +# Branching Strategy + +## Overview + +This project uses a **GitFlow-inspired** branching strategy for safe testing and releases. + +``` +Contributors → PR to dev → Preview Release → Merge to master → Production Release +``` + +## Branch Structure + +### `master` (Production) +- **Purpose**: Stable, production-ready code +- **Protection**: Protected branch, requires PR approval +- **Releases**: Only official releases (v1.0.0, v1.1.0, v1.2.0) +- **Merges from**: `dev` branch only (after testing) +- **Workflow**: `build-and-release-optimized.yml` (on version tags) + +### `dev` (Development/Integration) +- **Purpose**: Integration branch for testing new features +- **Protection**: Protected branch, requires PR approval +- **Releases**: Preview/beta releases (v1.2.0-dev-abc1234) +- **Merges from**: Feature branches, external PRs +- **Workflow**: `build-dev-preview.yml` (automatic on push) + +### Feature Branches +- **Naming**: `feature/description`, `fix/issue-number`, `enhancement/name` +- **Purpose**: Individual features or bug fixes +- **Created from**: `dev` branch +- **Merged to**: `dev` branch via PR +- **Lifetime**: Short-lived, deleted after merge + +## Workflow for Contributors + +### External Contributors (First-time or community) + +1. **Fork the repository** +2. **Create feature branch** from latest `master`: + ```bash + git checkout master + git pull origin master + git checkout -b feature/my-feature + ``` +3. **Make changes and commit** +4. **Create PR targeting `dev` branch** (not `master`!) + - PR will automatically trigger dev preview build + - Maintainer will review and test +5. **After approval**: PR merged to `dev` + +### Maintainers (Project owners) + +1. **Review PRs to `dev`**: + - Check security using `.github/PR_SECURITY_CHECKLIST.md` + - Test dev preview build from GitHub Actions + - Verify functionality + - Request changes if needed + +2. **Merge to `dev`**: + - Use "Squash and merge" for clean history + - Delete feature branch after merge + +3. **Test `dev` branch**: + - Use dev preview releases + - Gather feedback + - Fix any issues + +4. **Promote to `master`**: + ```bash + git checkout master + git pull origin master + git merge --no-ff dev -m "Release v1.2.0: Description" + git tag v1.2.0 + git push origin master --tags + ``` + - This triggers production release workflow + +## Release Process + +### Preview Releases (from `dev`) + +**Triggered by**: Push to `dev` branch or PR to `dev` + +**Version format**: `1.2.0-dev-abc1234` or `1.2.0-pr5-abc1234` + +**Steps**: +1. PR merged to `dev` (or direct push by maintainer) +2. GitHub Actions runs `build-dev-preview.yml` +3. Builds artifacts for x64 and ARM64 +4. Uploads to Actions artifacts (14-day retention) +5. Comments on PR with download links and testing checklist +6. **No GitHub Release created** (artifacts only) + +**Testing**: +- Download artifacts from Actions run +- Install locally and test +- Report issues as comments on PR or new issues +- If critical bugs found, fix in `dev` before promoting to `master` + +### Production Releases (from `master`) + +**Triggered by**: Version tag on `master` branch + +**Version format**: `1.2.0` (semver) + +**Steps**: +1. All testing completed on `dev` preview +2. Merge `dev` to `master` with `--no-ff` +3. Create version tag: + ```bash + git tag -a v1.2.0 -m "Release v1.2.0: Feature description" + git push origin v1.2.0 + ``` +4. GitHub Actions runs `build-and-release-optimized.yml` +5. Creates official GitHub Release with: + - Release notes + - x64 and ARM64 ZIPs + - SHA256 checksums +6. Updates README.md download links (manual) +7. Announces release (optional) + +## Branch Protection Rules + +### For `master`: +```yaml +Required reviews: 1 +Dismiss stale reviews: true +Require review from code owners: true +Require status checks: true + - Build (x64) + - Build (ARM64) +Require branches to be up to date: true +Include administrators: false +``` + +### For `dev`: +```yaml +Required reviews: 1 +Dismiss stale reviews: false +Require status checks: true + - Build (x64) + - Build (ARM64) +Include administrators: false +``` + +## Setting Up GitHub Branch Protection + +1. Go to **Settings** → **Branches** +2. Click **Add rule** for `master`: + - Branch name pattern: `master` + - ✅ Require pull request reviews before merging + - ✅ Require status checks to pass before merging + - Select: `build (x64)`, `build (ARM64)` + - ✅ Require branches to be up to date before merging + - ✅ Do not allow bypassing the above settings + +3. Click **Add rule** for `dev`: + - Branch name pattern: `dev` + - ✅ Require pull request reviews before merging + - ✅ Require status checks to pass before merging + - Select: `build (x64)`, `build (ARM64)` + +## Creating the `dev` Branch + +```bash +# From your current master branch +git checkout master +git pull origin master + +# Create dev branch +git checkout -b dev + +# Push to remote +git push -u origin dev + +# Set dev as default branch for PRs (optional) +# GitHub Settings → General → Default branch → dev +``` + +## Updating Workflows for Dev Branch + +The `build-dev-preview.yml` workflow already supports this strategy. It will: +- Build on push to `dev` branch +- Build on PRs targeting `dev` or `master` +- Create preview releases with proper versioning +- Comment on PRs with testing instructions + +## Example Scenario + +### Scenario: External user adds new feature + +1. **User creates PR**: + - User: `git checkout -b feature/streaming-window` + - User: Makes changes, commits + - User: Creates PR targeting `dev` branch + +2. **Auto preview build**: + - GitHub Actions builds preview + - Comments on PR: "Preview build ready: QuickAi-1.2.0-pr7-abc1234-x64.zip" + +3. **Maintainer reviews**: + - Reviews code using security checklist + - Downloads preview build from Actions + - Tests locally + - Approves PR + +4. **Merge to dev**: + - PR merged via "Squash and merge" + - Dev branch updated + - New preview build created: `QuickAi-1.2.0-dev-def5678-x64.zip` + +5. **Community testing**: + - Announce in discussions: "Please test dev preview build" + - Users download and test + - Report any bugs as issues + +6. **After testing period (e.g., 1 week)**: + - If stable: Maintainer merges `dev` → `master` + - Creates tag `v1.2.0` + - Production release published + - Users get stable update + +## Hotfix Process + +For critical bugs in production: + +```bash +# Create hotfix branch from master +git checkout master +git checkout -b hotfix/critical-bug + +# Fix and commit +git commit -m "fix: critical security issue" + +# Merge to master (fast-track) +git checkout master +git merge --no-ff hotfix/critical-bug +git tag v1.1.1 +git push origin master --tags + +# Also merge to dev to keep in sync +git checkout dev +git merge --no-ff hotfix/critical-bug +git push origin dev + +# Delete hotfix branch +git branch -d hotfix/critical-bug +``` + +## Version Numbering + +- **Major**: Breaking changes (v2.0.0) +- **Minor**: New features, backward compatible (v1.2.0) +- **Patch**: Bug fixes (v1.1.1) +- **Preview**: `-dev-shortsha` for dev builds +- **PR**: `-pr7-shortsha` for PR preview builds + +## FAQ + +**Q: Can I create PR directly to master?** +A: No, all PRs should target `dev` first for testing. + +**Q: When should I merge dev to master?** +A: After thorough testing (1-2 weeks) and no critical issues. + +**Q: Can I push directly to dev?** +A: Maintainers can, but PRs are preferred for code review. + +**Q: How long do preview builds stay available?** +A: 14 days (GitHub Actions artifact retention), then deleted automatically. + +**Q: Can I create a manual preview release?** +A: Yes, go to Actions → Build Dev Preview → Run workflow → Select branch. diff --git a/.github/workflows/build-dev-preview.yml b/.github/workflows/build-dev-preview.yml new file mode 100644 index 0000000..2c97330 --- /dev/null +++ b/.github/workflows/build-dev-preview.yml @@ -0,0 +1,226 @@ +name: Build Dev Preview + +on: + push: + branches: + - dev # Auto-build when dev branch updates + pull_request: + branches: + - dev # Build PRs targeting dev + - master # Build PRs targeting master (for hotfixes) + workflow_dispatch: + inputs: + branch: + description: 'Branch to build (leave empty for current)' + required: false + type: string + +permissions: + contents: write + pull-requests: write + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + DOTNET_VERSION: '9.0.x' + DOTNET_SKIP_FIRST_TIME_EXPERIENCE: 'true' + DOTNET_CLI_TELEMETRY_OPTOUT: 'true' + DOTNET_NOLOGO: 'true' + MSBUILDDISABLENODEREUSE: '1' + +jobs: + build: + runs-on: windows-latest + strategy: + matrix: + platform: [x64, ARM64] + fail-fast: false + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 # Full history for proper version detection + + - name: Cache NuGet packages + uses: actions/cache@v4 + with: + path: ~/.nuget/packages + key: ${{ runner.os }}-nuget-${{ hashFiles('**/*.csproj', '**/*.props') }} + restore-keys: | + ${{ runner.os }}-nuget- + + - name: Setup .NET + uses: actions/setup-dotnet@v4 + with: + dotnet-version: ${{ env.DOTNET_VERSION }} + + - name: Extract version (Dev Preview) + id: version + shell: pwsh + run: | + # Generate dev preview version + $baseVersion = "1.2.0" # Next version + $shortSha = "${{ github.sha }}".Substring(0,7) + $timestamp = Get-Date -Format "yyyyMMdd-HHmmss" + + $version = if ($env:GITHUB_EVENT_NAME -eq 'pull_request') { + $prNumber = "${{ github.event.pull_request.number }}" + "${baseVersion}-pr${prNumber}-${shortSha}" + } elseif ($env:GITHUB_REF -match 'refs/heads/(.+)') { + $branch = $matches[1] -replace '[/\\]', '-' + "${baseVersion}-${branch}-${shortSha}" + } else { + "${baseVersion}-dev-${shortSha}" + } + + "VERSION=$version" | Out-File -FilePath $env:GITHUB_OUTPUT -Append -Encoding utf8 + Write-Host "Dev Preview Version: $version" + + - name: Build with optimizations + shell: pwsh + run: | + $buildArgs = @( + 'QuickAi/QuickAi.sln', + '-c', 'Release', + '-p:Platform=${{ matrix.platform }}', + '-p:UseSharedCompilation=false', + '-p:BuildInParallel=true', + '-p:ContinuousIntegrationBuild=true', + '-p:Deterministic=true', + '-p:DebugType=embedded', # Keep symbols for dev builds + '-p:DebugSymbols=true', # Easier debugging + '--nologo', + '-v:minimal' + ) + dotnet build @buildArgs + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + + - name: Prepare artifacts (dev preview) + shell: pwsh + run: | + $ErrorActionPreference = 'Stop' + $version = '${{ steps.version.outputs.VERSION }}' + $platform = '${{ matrix.platform }}' + + $buildPath = "QuickAi/Community.PowerToys.Run.Plugin.QuickAi/bin/$platform/Release/net9.0-windows10.0.22621.0" + $artifactPath = "artifacts/QuickAi-v$version-$platform/QuickAi" + $zipPath = "artifacts/QuickAi-$version-$platform.zip" + + [void](New-Item -ItemType Directory -Force -Path $artifactPath) + + robocopy "$buildPath" "$artifactPath" /E /NFL /NDL /NJH /NJS /NC /NS /NP + if ($LASTEXITCODE -ge 8) { + Write-Error "Robocopy failed with exit code $LASTEXITCODE" + exit 1 + } + $LASTEXITCODE = 0 + + # Remove PowerToys dependencies + $filesToRemove = @( + 'PowerToys.Common.UI.dll', 'PowerToys.Common.UI.pdb', + 'PowerToys.ManagedCommon.dll', 'PowerToys.ManagedCommon.pdb', + 'PowerToys.Settings.UI.Lib.dll', 'PowerToys.Settings.UI.Lib.pdb', + 'Wox.Infrastructure.dll', 'Wox.Infrastructure.pdb', + 'Wox.Plugin.dll', 'Wox.Plugin.pdb' + ) + + foreach ($file in $filesToRemove) { + $filePath = Join-Path $artifactPath $file + if (Test-Path $filePath -PathType Leaf) { + [System.IO.File]::Delete($filePath) + } + } + + # Create ZIP + Add-Type -Assembly System.IO.Compression.FileSystem + $compressionLevel = [System.IO.Compression.CompressionLevel]::Optimal + [System.IO.Compression.ZipFile]::CreateFromDirectory( + (Resolve-Path "artifacts/QuickAi-v$version-$platform").Path, + (Join-Path (Get-Location) $zipPath), + $compressionLevel, + $false + ) + + Write-Host "✓ Created $zipPath" + + - name: Upload artifacts + uses: actions/upload-artifact@v4 + with: + name: quickai-dev-${{ matrix.platform }} + path: artifacts/*.zip + retention-days: 14 # Keep dev builds for 2 weeks + + comment-on-pr: + needs: build + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + + steps: + - name: Download artifacts + uses: actions/download-artifact@v4 + with: + path: artifacts + merge-multiple: true + + - name: Extract version + id: version + run: | + # Extract version from first zip filename + VERSION=$(ls artifacts/*.zip | head -1 | sed -E 's/.*QuickAi-([^-]+-pr[0-9]+-[a-f0-9]+)-.*/\1/') + echo "VERSION=$VERSION" >> $GITHUB_OUTPUT + + - name: Generate checksums + run: | + cd artifacts + for file in *.zip; do + sha256sum "$file" | awk '{print toupper($1) " " $2}' > "${file}.sha256" + done + + - name: Comment on PR + uses: actions/github-script@v7 + with: + script: | + const fs = require('fs'); + const path = require('path'); + + const artifactsDir = 'artifacts'; + const files = fs.readdirSync(artifactsDir).filter(f => f.endsWith('.zip')); + + let comment = `## 🔨 Dev Preview Build Ready\n\n`; + comment += `**Version:** \`${{ steps.version.outputs.VERSION }}\`\n`; + comment += `**Commit:** ${{ github.sha }}\n\n`; + comment += `### 📦 Download Test Builds\n\n`; + comment += `Download the artifacts from the [Actions run](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}) to test this PR.\n\n`; + + comment += `**Available builds:**\n`; + files.forEach(file => { + const platform = file.includes('x64') ? 'x64' : 'ARM64'; + comment += `- ${platform}: \`${file}\`\n`; + }); + + comment += `\n### 📋 Testing Checklist\n\n`; + comment += `- [ ] Download and extract the build for your platform\n`; + comment += `- [ ] Copy to \`%LOCALAPPDATA%\\Microsoft\\PowerToys\\PowerToys Run\\Plugins\\QuickAi\\\`\n`; + comment += `- [ ] Restart PowerToys\n`; + comment += `- [ ] Test basic query functionality\n`; + comment += `- [ ] Test streaming window (if applicable)\n`; + comment += `- [ ] Test theme switching (Dark/Light)\n`; + comment += `- [ ] Check for crashes or errors\n`; + + comment += `\n### ⚠️ Security Review Required\n\n`; + comment += `Before merging, ensure:\n`; + comment += `- [ ] No XSS vulnerabilities in XAML/UI rendering\n`; + comment += `- [ ] Proper input sanitization\n`; + comment += `- [ ] No unsafe Dispatcher.Invoke usage\n`; + comment += `- [ ] Thread-safe UI updates\n`; + comment += `- [ ] Proper resource disposal\n`; + + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: comment + }); diff --git a/SETUP_DEV_BRANCH.md b/SETUP_DEV_BRANCH.md new file mode 100644 index 0000000..0c2ca5d --- /dev/null +++ b/SETUP_DEV_BRANCH.md @@ -0,0 +1,249 @@ +# Setting Up Dev Branch Workflow + +Quick guide for setting up the `dev` branch and GitFlow workflow. + +## Step 1: Create Dev Branch + +```bash +# Make sure you're on latest master +git checkout master +git pull origin master + +# Create and push dev branch +git checkout -b dev +git push -u origin dev +``` + +## Step 2: Configure Branch Protection on GitHub + +### Protect `master` branch: +1. Go to **Settings** → **Branches** → **Add rule** +2. Branch name pattern: `master` +3. Settings: + - ✅ Require a pull request before merging + - ✅ Require approvals: 1 + - ✅ Dismiss stale pull request approvals when new commits are pushed + - ✅ Require status checks to pass before merging + - ✅ Require branches to be up to date before merging + - Status checks: Wait for workflows to run, then select: + - `build (x64)` + - `build (ARM64)` + - ✅ Require conversation resolution before merging + - ✅ Do not allow bypassing the above settings +4. **Save changes** + +### Protect `dev` branch: +1. Go to **Settings** → **Branches** → **Add rule** +2. Branch name pattern: `dev` +3. Settings: + - ✅ Require a pull request before merging + - ✅ Require approvals: 1 + - ✅ Require status checks to pass before merging + - Status checks: Select same as above + - ✅ Require conversation resolution before merging +4. **Save changes** + +## Step 3: Update GitHub PR Settings + +1. Go to **Settings** → **General** → **Pull Requests** +2. Configure: + - ✅ Allow squash merging (recommended for cleaner history) + - Default to pull request title and description + - ✅ Allow merge commits (for dev → master) + - ❌ Allow rebase merging (optional) + - ✅ Automatically delete head branches + +## Step 4: Set Default Branch for PRs (Optional) + +If you want external PRs to target `dev` by default: + +1. Go to **Settings** → **General** → **Default branch** +2. Click "Switch to another branch" +3. Select `dev` +4. Confirm + +**Note**: This means new PRs will default to `dev`, but you can still create PRs to `master` manually. + +## Step 5: Update README.md + +Add a section explaining the contribution workflow: + +```markdown +## Contributing + +We use a dev branch workflow: +- All PRs should target the `dev` branch +- After testing, changes are promoted to `master` +- Preview builds are available for testing + +See [BRANCHING_STRATEGY.md](.github/BRANCHING_STRATEGY.md) for details. +``` + +## Step 6: Test the Workflow + +### Test dev preview build: + +```bash +# Create a test branch +git checkout dev +git checkout -b test/preview-workflow + +# Make a small change +echo "# Test" >> TEST.md +git add TEST.md +git commit -m "test: verify dev preview workflow" +git push -u origin test/preview-workflow + +# Create PR to dev branch on GitHub +# Verify that: +# 1. Build Dev Preview workflow runs +# 2. Artifacts are created +# 3. PR comment appears with download links +``` + +### Test production release: + +```bash +# After testing dev branch +git checkout master +git merge --no-ff dev -m "Release v1.2.0-test" +git tag v1.2.0-test +git push origin master --tags + +# Verify that: +# 1. Build and Release workflow runs +# 2. GitHub Release is created +# 3. Release has correct assets + +# Clean up test release +git tag -d v1.2.0-test +git push origin :refs/tags/v1.2.0-test +``` + +## Step 7: Communicate to Contributors + +Add to issue/PR templates: + +**For `.github/PULL_REQUEST_TEMPLATE.md`:** + +```markdown +## PR Checklist + +- [ ] This PR targets the `dev` branch (not `master`) +- [ ] I have tested my changes locally +- [ ] I have added tests (if applicable) +- [ ] I have updated documentation (if needed) + +**Note**: PRs are first merged to `dev` for preview testing, then promoted to `master` after verification. +``` + +## Step 8: Set Up Auto-Labeling (Optional) + +Create `.github/labeler.yml`: + +```yaml +preview-ready: + - changed-files: + - any-glob-to-any-file: '**/*' + - base-branch: 'dev' + +security-review-needed: + - changed-files: + - any-glob-to-any-file: + - '**/*.xaml' + - '**/Main.cs' + - '**/*Window*.cs' +``` + +And workflow `.github/workflows/labeler.yml`: + +```yaml +name: "Pull Request Labeler" +on: + pull_request_target: + types: [opened, synchronize] + +jobs: + label: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + steps: + - uses: actions/labeler@v5 +``` + +## Troubleshooting + +### Issue: Workflows not running on dev branch + +**Solution**: Ensure `.github/workflows/build-dev-preview.yml` has: +```yaml +on: + push: + branches: + - dev +``` + +### Issue: Can't push to protected branch + +**Expected**: Protection is working correctly. Use PRs instead: +```bash +git checkout -b fix/my-fix +git push -u origin fix/my-fix +# Create PR on GitHub +``` + +### Issue: Status checks not showing + +**Solution**: +1. Trigger at least one workflow run +2. Then they'll appear in branch protection settings +3. Go back and enable them + +## Current PR Review Process + +For the existing PR (streaming window feature): + +```bash +# 1. Fetch PR branch (if it exists) +git fetch origin pull/ID/head:pr-streaming-window + +# 2. Review code locally +git checkout pr-streaming-window + +# 3. Check security using checklist +# See .github/PR_SECURITY_CHECKLIST.md + +# 4. If approved, merge to dev +git checkout dev +git merge --no-ff pr-streaming-window -m "Merge PR: Add streaming results window" +git push origin dev + +# 5. Test dev preview build +# Download from GitHub Actions +# Test locally + +# 6. After 1-2 weeks of testing, promote to master +git checkout master +git merge --no-ff dev -m "Release v1.2.0: Streaming results window" +git tag v1.2.0 +git push origin master --tags +``` + +## Summary + +✅ Dev branch workflow protects master from untested changes +✅ Preview builds allow community testing before release +✅ Clear promotion path: feature → dev → master +✅ Automated builds on every PR +✅ Security review checklist for reviewers + +--- + +**Next Steps:** +1. Run steps 1-6 above +2. Review existing PR with security checklist +3. Merge to dev (not master) +4. Test dev preview +5. Promote to master when ready From dbea532ee267c64bcee62f695b4e6b3ab9b7ea0e Mon Sep 17 00:00:00 2001 From: ruslanlap Date: Mon, 29 Dec 2025 16:44:55 +0200 Subject: [PATCH 2/2] docs: add CLAUDE.md and security checklist --- .github/PR_SECURITY_CHECKLIST.md | 204 +++++++++++++++++++ .gitignore | 2 + CLAUDE.md | 323 +++++++++++++++++++++++++++++++ 3 files changed, 529 insertions(+) create mode 100644 .github/PR_SECURITY_CHECKLIST.md create mode 100644 CLAUDE.md diff --git a/.github/PR_SECURITY_CHECKLIST.md b/.github/PR_SECURITY_CHECKLIST.md new file mode 100644 index 0000000..3218f44 --- /dev/null +++ b/.github/PR_SECURITY_CHECKLIST.md @@ -0,0 +1,204 @@ +# PR Security Review Checklist + +Use this checklist when reviewing PRs, especially those that add new UI components or handle user input. + +## 🔒 Critical Security Checks + +### 1. XAML/WPF UI Security + +#### XSS and Code Injection +- [ ] **No dynamic XAML parsing**: Ensure no `XamlReader.Parse()` or `XamlReader.Load()` with user-controlled strings +- [ ] **No dynamic code execution**: Check for `Assembly.Load()`, `Activator.CreateInstance()` with user input +- [ ] **Safe data binding**: Verify all bindings use proper value converters and validation +- [ ] **Command injection**: No shell commands constructed from user input without sanitization + +#### Content Rendering +- [ ] **FlowDocument/RichTextBox safety**: If using FlowDocument, ensure: + - No `Hyperlink` navigation to arbitrary URLs without validation + - No JavaScript injection via WebBrowser control + - Proper encoding of AI response text before rendering +- [ ] **HTML rendering**: If rendering HTML, use safe libraries and sanitize content +- [ ] **Image loading**: Validate image sources, avoid loading from untrusted URLs + +### 2. Thread Safety + +#### Dispatcher Usage +- [ ] **Correct Dispatcher.Invoke usage**: + ```csharp + // ✅ GOOD: Async, non-blocking + Dispatcher.BeginInvoke(() => { ... }, DispatcherPriority.Background); + + // ❌ BAD: Synchronous, can deadlock + Dispatcher.Invoke(() => { ... }); + ``` +- [ ] **No deadlocks**: Ensure UI thread never waits synchronously on background thread +- [ ] **Proper priority**: Use `DispatcherPriority.Background` for non-critical updates +- [ ] **Exception handling**: Wrap Dispatcher calls in try-catch to prevent crashes + +#### Race Conditions +- [ ] **Proper locking**: Check all shared state access is protected by locks +- [ ] **Thread-safe collections**: Use `ConcurrentQueue`, `ConcurrentDictionary` if needed +- [ ] **Cancellation tokens**: Verify proper `CancellationToken` usage in async methods +- [ ] **No torn reads/writes**: Ensure atomic operations for shared variables + +### 3. Input Validation and Sanitization + +#### AI Response Handling +- [ ] **Length limits**: Enforce maximum response length to prevent memory exhaustion +- [ ] **Character encoding**: Properly handle UTF-8, emoji, special characters +- [ ] **Null/empty checks**: Validate all external input before processing +- [ ] **Malicious content**: Consider filtering dangerous patterns (if applicable) + +#### User Input +- [ ] **Query validation**: Sanitize user queries before sending to API +- [ ] **Settings validation**: Validate all settings (URLs, tokens, numbers) before use +- [ ] **Path traversal**: If handling file paths, prevent directory traversal attacks + +### 4. Resource Management + +#### Memory Leaks +- [ ] **Event unsubscription**: All `+=` event handlers have matching `-=` in Dispose +- [ ] **IDisposable implementation**: Proper Dispose pattern for: + - Windows/controls + - Event subscriptions + - Timers + - HTTP clients + - Streams +- [ ] **No circular references**: Check for object graphs that prevent GC +- [ ] **StringBuilder capacity**: Set initial capacity to avoid reallocations + +#### Window Management +- [ ] **Window disposal**: ResultsWindow properly disposed after closing +- [ ] **Singleton pattern**: Only one results window per query (avoid duplicates) +- [ ] **Owner window**: Set `Owner` property to prevent orphaned windows +- [ ] **ShowDialog vs Show**: Use appropriate method to control lifetime + +### 5. Error Handling + +#### Exception Safety +- [ ] **Try-catch blocks**: Wrap risky operations (networking, UI updates, parsing) +- [ ] **User-friendly errors**: Don't expose stack traces to users +- [ ] **Logging**: Log errors for debugging without exposing sensitive data +- [ ] **Graceful degradation**: Plugin continues working even if one feature fails + +#### Edge Cases +- [ ] **Null reference protection**: Use null-conditional operators (`?.`, `??`) +- [ ] **Empty responses**: Handle empty/whitespace-only AI responses +- [ ] **Network failures**: Proper timeout and retry logic +- [ ] **Window closed during streaming**: Handle window disposal mid-stream + +### 6. Performance and DoS Protection + +#### Resource Limits +- [ ] **Streaming throttling**: Batch UI updates (e.g., every 3 tokens or 150ms) +- [ ] **Max tokens**: Enforce reasonable limits (prevent huge responses) +- [ ] **Timeout protection**: All HTTP requests have timeouts +- [ ] **Concurrent requests**: Limit number of simultaneous queries + +#### UI Responsiveness +- [ ] **No UI blocking**: All heavy operations on background threads +- [ ] **Smooth scrolling**: Auto-scroll doesn't freeze UI +- [ ] **Animation performance**: Theme transitions don't lag +- [ ] **Memory usage**: Monitor memory during long streaming sessions + +## 🎯 Specific Checks for ResultsWindow PR + +Based on the PR description, check these specific items: + +### New ResultsWindow.xaml +- [ ] **FlowDocument content source**: How is AI text added to FlowDocument? + - Using `Run` elements (safe) or dynamic XAML (unsafe)? + - Proper escaping of special characters? +- [ ] **Hyperlink handling**: If supporting hyperlinks, validate URLs before navigation +- [ ] **Resource dictionary**: Theme resources properly scoped, no global pollution +- [ ] **Window properties**: Proper `SizeToContent`, `ResizeMode`, `WindowStartupLocation` + +### StreamingSession.TokenReceived Event +- [ ] **Event subscription**: Where is `TokenReceived +=` added? Is it unsubscribed? +- [ ] **Event args**: Does event pass sanitized data or raw API response? +- [ ] **Thread context**: Is event raised on UI thread or background thread? +- [ ] **Exception propagation**: What happens if event handler throws? + +### Dispatcher.BeginInvoke Usage +- [ ] **Correct priority**: Using `Background` or `Normal`, not `Send`? +- [ ] **No closure captures**: Avoid capturing large objects in lambda +- [ ] **Exception handling**: Try-catch inside Dispatcher callback +- [ ] **Cancellation check**: Verify window still exists before updating + +### Theme Support +- [ ] **Theme resources**: Using PowerToys theme API correctly? +- [ ] **Dynamic updates**: Theme change event handler properly registered/unregistered +- [ ] **Resource lookup**: Fallback values if theme resource missing +- [ ] **Window already open**: What happens if theme changes while window is visible? + +### Error Handling in TriggerRefresh +- [ ] **What error was being thrown**: Understanding root cause +- [ ] **Proper fix**: Is try-catch the right solution or symptom fix? +- [ ] **Window disposal check**: Verify window exists before calling API +- [ ] **Null reference safety**: Check `_context?.API?.ChangeQuery()` pattern + +## 📋 Code Review Process + +### Step 1: Read the Diff +- [ ] Review all changed files line by line +- [ ] Identify new classes, methods, events +- [ ] Note any changed access modifiers or visibility + +### Step 2: Check Dependencies +- [ ] Any new NuGet packages? Verify they're trusted and updated +- [ ] New assembly references? +- [ ] Check for supply chain risks + +### Step 3: Test Locally +- [ ] Build the PR branch locally +- [ ] Test streaming functionality +- [ ] Test theme switching +- [ ] Test edge cases (close window mid-stream, network failure, etc.) +- [ ] Check Task Manager for memory leaks +- [ ] Test on both x64 and ARM64 (if possible) + +### Step 4: Security Scan +- [ ] Run static analysis (if available) +- [ ] Check for common CWE patterns +- [ ] Review crypto usage (if any) +- [ ] Check for hardcoded secrets or API keys + +### Step 5: Performance Test +- [ ] Long streaming responses (1000+ tokens) +- [ ] Rapid queries (open/close window quickly) +- [ ] Theme switching during streaming +- [ ] Multiple windows (if allowed) + +## 🚨 Red Flags (Immediate Review Required) + +- ❌ Any use of `eval()`, `Invoke()`, `DynamicInvoke()` +- ❌ `Process.Start()` with user input +- ❌ SQL queries with string concatenation +- ❌ Deserializing untrusted data without validation +- ❌ `AllowUnsafeHeaderParsing = true` +- ❌ Disabled SSL certificate validation +- ❌ Hardcoded credentials or API keys +- ❌ Unbounded loops or recursion +- ❌ File I/O without path validation +- ❌ Registry access without validation + +## ✅ Approval Criteria + +Before approving PR: +- [ ] All critical security checks passed +- [ ] No memory leaks detected in testing +- [ ] UI remains responsive during streaming +- [ ] Theme switching works correctly +- [ ] No exceptions in normal operation +- [ ] Edge cases handled gracefully +- [ ] Code follows existing patterns +- [ ] Proper error messages for users +- [ ] Documentation updated (if needed) +- [ ] CLAUDE.md updated (if architecture changed) + +## 📚 References + +- [Microsoft WPF Security Guidelines](https://learn.microsoft.com/en-us/dotnet/desktop/wpf/security-wpf) +- [OWASP Top 10](https://owasp.org/www-project-top-ten/) +- [CWE Top 25](https://cwe.mitre.org/top25/) +- [PowerToys Security Policy](https://github.com/microsoft/PowerToys/security/policy) diff --git a/.gitignore b/.gitignore index 90434dc..df270dd 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,8 @@ bugfix *.sln.docstates CLAUDE.md PERFORMANCE_RECOMMENDATIONS.md +PR_SECURITY_CHECKLIST.md + QUICK_WINS.md SUMMARY_UA.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..23724fd --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,323 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project Overview + +QuickAI is a PowerToys Run plugin that provides AI-powered assistance directly in the Windows PowerToys launcher. Users can type `ai ` and get streaming responses from multiple AI providers (Groq, Together, Fireworks, OpenRouter, Cohere, Google, Ollama, and OpenAI Compatible endpoints). + +**Key Information:** +- Plugin ID: `420129A62ECA49848C5C7CA229BFD22C` +- Default action keyword: `ai` +- Target: .NET 9.0 for Windows 10.0.22621.0 +- Main implementation: Single file `QuickAi/Community.PowerToys.Run.Plugin.QuickAi/Main.cs` (~1320 lines) + +## Build and Development Commands + +### Building the plugin +```bash +# Build for both x64 and ARM64, create distribution ZIPs +./build-and-zip.sh + +# Or use .NET directly for a specific platform +cd QuickAi +dotnet build QuickAi.sln -c Release -p:Platform=x64 +dotnet build QuickAi.sln -c Release -p:Platform=ARM64 + +# Publish for distribution +dotnet publish Community.PowerToys.Run.Plugin.QuickAi/Community.PowerToys.Run.Plugin.QuickAi.csproj \ + -c Release -r win-x64 --self-contained false +``` + +### Running tests +```bash +cd QuickAi +dotnet test Community.PowerToys.Run.Plugin.QuickAi.UnitTests/Community.PowerToys.Run.Plugin.QuickAi.UnitTests.csproj +``` + +### Local development +```bash +# Build and manually copy to PowerToys plugin directory +dotnet build -c Release -p:Platform=x64 +# Then copy output from bin/x64/Release/net9.0-windows10.0.22621.0/ +# to %LOCALAPPDATA%\Microsoft\PowerToys\PowerToys Run\Plugins\QuickAi\ +``` + +## Architecture + +### Core Plugin Interface Implementation + +The `Main` class implements four PowerToys interfaces: +- **IPlugin**: Core plugin functionality (Init, Query) +- **ISettingProvider**: Exposes settings UI via `AdditionalOptions` property +- **IContextMenu**: Right-click context menu (show full response, copy, restart) +- **IDisposable**: Proper cleanup of HTTP resources and streaming sessions + +### Multi-Provider Architecture + +**Provider Configuration (`ProviderConfigurations` dictionary):** +- Maps provider name → endpoint URL + schema type +- Schema types: `OpenAI`, `Cohere`, `Google` (different JSON payload formats) +- Ollama and OpenAI Compatible providers use OpenAI schema + +**API Request Flow:** +1. User types query → `Query()` method called by PowerToys +2. Query validation (check for API key, prompt) +3. `StartQuery()` creates a `StreamingSession` +4. `StreamWithConfigurationAsync()` builds HTTP request based on provider schema +5. Server-Sent Events (SSE) stream parsed line-by-line +6. Delta extraction logic differs by provider (OpenAI vs Cohere vs Google) +7. UI refreshed via `_context.API.ChangeQuery()` callback mechanism + +### Streaming Response System + +**Key Components:** +- **StreamingSession class**: Manages per-query state (prompt, response builder, cancellation) +- **Batched UI updates**: Refreshes UI every 3 tokens OR 150ms to avoid performance issues +- **Thread safety**: All session access protected by `_sessionGate` lock +- **Cancellation**: When user types new query, previous session cancelled via `CancellationTokenSource` + +**Performance optimizations:** +- Static `HttpClient` singleton with HTTP/2 support +- Connection pooling (5 min lifetime, 2 min idle timeout) +- Brotli/Gzip/Deflate compression support +- TLS 1.2/1.3 only +- Configurable timeout (default 8s, range 3-30s) + +### API Key Management + +**Dual key system:** +- Primary API key: First attempt +- Secondary API key: Automatic fallback on primary failure +- Provider-specific authentication: + - Google: `x-goog-api-key` header + - Ollama: No authentication + - Others: `Authorization: Bearer` header + +### Settings Configuration + +Settings stored in PowerToys settings JSON (`%LOCALAPPDATA%\Microsoft\PowerToys\PowerToys Run\settings.json`). + +**Configurable options:** +- Provider selection (dropdown) +- Primary/Secondary API keys (textbox) +- Model name (textbox) +- Max tokens (16-4096, default 128) +- Temperature (0.0-2.0, default 0.2) +- Request timeout (3-30 seconds, default 8) +- Ollama host URL (for Ollama provider) +- OpenAI Compatible endpoint URL + +**Important:** Settings use plain text storage - API keys are NOT encrypted. + +## Critical Implementation Details + +### Provider Schema Handling + +Three distinct schemas must be handled: + +1. **OpenAI Schema** (Groq, Together, Fireworks, OpenRouter, Ollama, OpenAI Compatible): +```json +{ + "model": "...", + "messages": [{"role": "user", "content": "..."}], + "stream": true, + "temperature": 0.2, + "max_tokens": 128 +} +``` +Delta extraction: `root.choices[0].delta.content` + +2. **Cohere Schema**: +```json +{ + "model": "command", + "message": "...", + "stream": true, + "temperature": 0.2, + "max_tokens": 128 +} +``` +Delta extraction: `root.event_type == "text-generation"` → `root.text` + +3. **Google Schema** (Gemini): +```json +{ + "contents": [{"parts": [{"text": "..."}]}], + "generationConfig": { + "temperature": 0.2, + "maxOutputTokens": 128 + } +} +``` +Endpoint includes model: `/v1beta/models/{model}:streamGenerateContent?key={apiKey}&alt=sse` +Delta extraction: `root.candidates[0].content.parts[0].text` + +### SSE Stream Parsing + +Streaming responses use Server-Sent Events format: +``` +data: {"choices":[{"delta":{"content":"Hello"}}]} + +data: {"choices":[{"delta":{"content":" world"}}]} + +data: [DONE] +``` + +Parser logic (`StreamWithConfigurationAsync`): +- Read line-by-line from response stream +- Lines starting with `"data: "` contain JSON payloads +- `"data: [DONE]"` signals end of stream +- Empty lines are field separators (ignored) +- Extract delta based on provider schema type +- Build cumulative response in `StringBuilder` + +### UI Refresh Mechanism + +PowerToys Run doesn't support live UI updates. Workaround: +1. Plugin calls `_context.API.ChangeQuery(rawQuery, true)` +2. This triggers PowerToys to re-call `Query()` method +3. `Query()` returns updated `StreamingSession.BuildResult()` with latest response +4. Flag `_uiRefreshPending` prevents infinite loops + +### Context Menu Integration + +Right-click on result shows: +- **Show full response (Enter)**: Opens WPF MessageBox with full text +- **Copy response (Ctrl+C)**: Copies to clipboard +- **Restart query (Ctrl+R)**: Re-runs the same prompt + +Context data stored in `Result.ContextData` as the full response string. + +## File Structure + +``` +PowerToysRun-QuickAi/ +├── QuickAi/ +│ ├── Community.PowerToys.Run.Plugin.QuickAi/ +│ │ ├── Main.cs # Entire plugin logic (1320 lines) +│ │ ├── plugin.json # Plugin manifest +│ │ ├── *.csproj # Project file +│ │ └── Images/ +│ │ ├── ai.dark.png # Dark theme icon +│ │ └── ai.light.png # Light theme icon +│ ├── Community.PowerToys.Run.Plugin.QuickAi.UnitTests/ +│ │ ├── MainTests.cs # Basic unit tests +│ │ └── *.csproj +│ └── QuickAi.sln # Main solution +├── build-and-zip.sh # Build script (creates release ZIPs) +├── .github/workflows/ +│ └── build-and-release-optimized.yml # CI/CD workflow +└── README.md +``` + +**Note:** The `src/` and `Templates.sln` are for a separate PowerToys plugin template project, not part of QuickAI plugin itself. + +## Build Output & Packaging + +Build outputs go to: +- `QuickAi/Community.PowerToys.Run.Plugin.QuickAi/bin/Release/net9.0-windows10.0.22621.0/{platform}/` + +Packaging (`build-and-zip.sh`) excludes PowerToys-provided dependencies: +- `PowerToys.Common.UI.*` +- `PowerToys.ManagedCommon.*` +- `PowerToys.Settings.UI.Lib.*` +- `Wox.Infrastructure.*` +- `Wox.Plugin.*` + +Final ZIPs created at repo root: `QuickAi-{version}-{platform}.zip` + +## CI/CD Pipeline + +GitHub Actions workflow (`.github/workflows/build-and-release-optimized.yml`): +- Triggers on version tags (`v*`) or manual dispatch +- Matrix build: x64 and ARM64 in parallel +- Optimizations: + - Shallow checkout with sparse-checkout + - NuGet package caching + - Parallel builds (`BuildInParallel=true`) + - Skip debug symbols (`DebugType=none`) + - Robocopy for fast file copying + - .NET compression APIs instead of `Compress-Archive` +- Creates GitHub release with checksums + +## Version Management + +Version controlled in `plugin.json`: +```json +{ + "Version": "1.1.0" +} +``` + +Build script extracts version via `sed`: +```bash +VERSION="$(sed -n 's/.*"Version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "$PLUGIN_JSON")" +``` + +## Adding a New AI Provider + +To add a new provider: + +1. Add provider constant in `Main.cs`: +```csharp +private const string ProviderNewProvider = "NewProvider"; +``` + +2. Add to `SupportedProviders` list + +3. Add to `ProviderConfigurations` dictionary: +```csharp +[ProviderNewProvider] = new("https://api.example.com/v1/chat", ProviderSchemaType.OpenAI) +``` + +4. If using non-OpenAI schema: + - Add new `ProviderSchemaType` enum value + - Implement payload building in `BuildHttpRequest()` + - Implement delta extraction in new method (similar to `ExtractOpenAiDelta`) + - Add case in SSE stream parsing logic + +5. Handle any special authentication in `BuildHttpRequest()` + +6. Update README.md with provider details + +## Common Patterns + +### Adding a new setting + +1. Add constant key: +```csharp +private const string NewSettingKey = "quickai_new_setting"; +``` + +2. Add field to store value: +```csharp +private string _newSetting = "default"; +``` + +3. Add to `AdditionalOptions` property (see existing examples) + +4. Parse in `UpdateSettings()` method + +### Error handling + +Always set user-friendly errors via: +```csharp +session.SetError("User-friendly error message"); +TriggerRefresh(session.RawQuery); +``` + +### Testing streaming locally + +Ollama is best for local testing: +```bash +ollama serve +ollama pull llama3.2 +# Configure plugin to use Ollama provider with model "llama3.2" +``` + +## Known Issues + +- API keys stored in plain text in PowerToys settings JSON (issue noted in README) +- "Show full response" modal may show partial text if response was truncated by low max_tokens or timeout (issue #3) +- Plugin currently doesn't support conversation history (stateless queries)