diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml new file mode 100644 index 00000000..7d6ab7c3 --- /dev/null +++ b/.github/workflows/codeql-analysis.yml @@ -0,0 +1,58 @@ +--- +name: CodeQL + +on: + push: + branches: + - main + - dev + paths: + - "**.cs" + - "**.csproj" + + pull_request: + branches: + - "**" + + workflow_dispatch: + + schedule: + - cron: "31 0 * * 5" + +jobs: + analyze: + name: Analyze + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write + + env: + CI: true + + strategy: + fail-fast: false + matrix: + language: ["csharp"] + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Setup .NET + uses: actions/setup-dotnet@v4 + with: + dotnet-version: '10.0.x' + dotnet-quality: 'preview' + + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: ${{ matrix.language }} + + - name: Autobuild + uses: github/codeql-action/autobuild@v3 + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 diff --git a/.github/workflows/squad-label-enforce.yml b/.github/workflows/squad-label-enforce.yml new file mode 100644 index 00000000..8ba133fd --- /dev/null +++ b/.github/workflows/squad-label-enforce.yml @@ -0,0 +1,176 @@ +--- +name: Squad Label Enforce + +on: + issues: + types: [labeled] + +permissions: + issues: write + contents: read + +jobs: + enforce: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Enforce mutual exclusivity + uses: actions/github-script@v7 + with: + script: | + const issue = context.payload.issue; + const appliedLabel = context.payload.label.name; + + // Namespaces with mutual exclusivity rules + const EXCLUSIVE_PREFIXES = ['go:', 'release:', 'type:', 'priority:']; + + // Skip if not a managed namespace label + if (!EXCLUSIVE_PREFIXES.some(p => appliedLabel.startsWith(p))) { + core.info(`Label ${appliedLabel} is not in a managed namespace — skipping`); + return; + } + + const allLabels = issue.labels.map(l => l.name); + + // Handle go: namespace (mutual exclusivity) + if (appliedLabel.startsWith('go:')) { + const otherGoLabels = allLabels.filter(l => + l.startsWith('go:') && l !== appliedLabel + ); + + if (otherGoLabels.length > 0) { + for (const label of otherGoLabels) { + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + name: label + }); + core.info(`Removed conflicting label: ${label}`); + } + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + body: `🏷️ Triage verdict updated → \`${appliedLabel}\`` + }); + } + + if (appliedLabel === 'go:yes') { + const hasReleaseLabel = allLabels.some(l => l.startsWith('release:')); + if (!hasReleaseLabel) { + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + labels: ['release:backlog'] + }); + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + body: `📋 Marked as \`release:backlog\` — assign a release target when ready.` + }); + + core.info('Applied release:backlog for go:yes issue'); + } + } + + if (appliedLabel === 'go:no') { + const releaseLabels = allLabels.filter(l => l.startsWith('release:')); + if (releaseLabels.length > 0) { + for (const label of releaseLabels) { + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + name: label + }); + core.info(`Removed release label from go:no issue: ${label}`); + } + } + } + } + + // Handle release: namespace (mutual exclusivity) + if (appliedLabel.startsWith('release:')) { + const otherReleaseLabels = allLabels.filter(l => + l.startsWith('release:') && l !== appliedLabel + ); + + if (otherReleaseLabels.length > 0) { + for (const label of otherReleaseLabels) { + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + name: label + }); + core.info(`Removed conflicting label: ${label}`); + } + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + body: `🏷️ Release target updated → \`${appliedLabel}\`` + }); + } + } + + // Handle type: namespace (mutual exclusivity) + if (appliedLabel.startsWith('type:')) { + const otherTypeLabels = allLabels.filter(l => + l.startsWith('type:') && l !== appliedLabel + ); + + if (otherTypeLabels.length > 0) { + for (const label of otherTypeLabels) { + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + name: label + }); + core.info(`Removed conflicting label: ${label}`); + } + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + body: `🏷️ Issue type updated → \`${appliedLabel}\`` + }); + } + } + + // Handle priority: namespace (mutual exclusivity) + if (appliedLabel.startsWith('priority:')) { + const otherPriorityLabels = allLabels.filter(l => + l.startsWith('priority:') && l !== appliedLabel + ); + + if (otherPriorityLabels.length > 0) { + for (const label of otherPriorityLabels) { + await github.rest.issues.removeLabel({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + name: label + }); + core.info(`Removed conflicting label: ${label}`); + } + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue.number, + body: `🏷️ Priority updated → \`${appliedLabel}\`` + }); + } + } + + core.info(`Label enforcement complete for ${appliedLabel}`); diff --git a/.github/workflows/squad-pr-auto-label.yml b/.github/workflows/squad-pr-auto-label.yml new file mode 100644 index 00000000..c4affe63 --- /dev/null +++ b/.github/workflows/squad-pr-auto-label.yml @@ -0,0 +1,94 @@ +--- +name: Squad PR Auto-Label + +on: + pull_request_target: + types: [opened, reopened, synchronize] + +permissions: + pull-requests: write + contents: read + +jobs: + auto-label: + runs-on: ubuntu-latest + steps: + - name: Auto-label PR for squad system + uses: actions/github-script@v7 + with: + script: | + const pr = context.payload.pull_request; + const author = pr.user.login; + + // Fetch current labels on the PR + const { data: currentLabels } = await github.rest.issues.listLabelsOnIssue({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pr.number + }); + + const labelNames = currentLabels.map(l => l.name); + + // Check if already has squad labels + const hasSquadLabel = labelNames.some(name => + name === 'squad' || name.startsWith('squad:') + ); + + if (hasSquadLabel) { + core.info(`PR #${pr.number} already has squad label(s) — skipping`); + return; + } + + let labelsToAdd = []; + let commentBody = ''; + + // Handle known automation bots + const knownBots = ['dependabot[bot]', 'renovate[bot]', 'github-actions[bot]']; + if (knownBots.includes(author)) { + labelsToAdd = ['squad:boromir', 'squad']; + commentBody = [ + `### 🤖 Dependency Update PR`, + '', + `This PR was opened by **${author}** and has been automatically labeled for **Boromir** (DevOps) to review.`, + '', + `**Labels applied:**`, + `- \`squad:boromir\` — Assigned to DevOps for dependency updates`, + `- \`squad\` — In triage queue`, + '', + `> Dependency and infrastructure updates are owned by the DevOps team.` + ].join('\n'); + } else { + // Handle general PRs without squad labels + labelsToAdd = ['squad']; + commentBody = [ + `### 🏗️ PR Added to Squad Triage Queue`, + '', + `This PR has been labeled with \`squad\` and added to the triage queue.`, + '', + `**Next steps:**`, + `- The squad Lead will review and assign to an appropriate team member`, + `- A \`squad:member\` label will be added after triage`, + '', + `> If you know which squad member should handle this, you can add the appropriate \`squad:member\` label yourself.` + ].join('\n'); + } + + // Add labels + await github.rest.issues.addLabels({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pr.number, + labels: labelsToAdd + }); + + core.info(`Added labels to PR #${pr.number}: ${labelsToAdd.join(', ')}`); + + // Post comment + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pr.number, + body: commentBody + }); + + core.info(`Posted auto-label comment on PR #${pr.number}`); diff --git a/.squad/agents/gandalf/history.md b/.squad/agents/gandalf/history.md index d91d76c4..88b0d564 100644 --- a/.squad/agents/gandalf/history.md +++ b/.squad/agents/gandalf/history.md @@ -2,6 +2,84 @@ ## Learnings +### PR #5, #6, #7 Security Review — 2026-04-18 + +**PRs Reviewed:** +- **PR #5:** "ci: add PR build and test workflow" ✅ MERGED +- **PR #6:** "chore: remove Weather and Counter template leftovers" ✅ MERGED +- **PR #7:** "chore: add copyright headers to all .cs files" ✅ MERGED + +**PR #5 Security Assessment (CI Workflow):** + +Files changed: `.github/workflows/ci.yml`, `.squad/agents/boromir/history.md` + +Security findings: +- ✅ **No hardcoded secrets** — All authentication uses GitHub tokens with proper scopes +- ✅ **Minimal RBAC permissions** — `contents:read`, `checks:write`, `pull-requests:write` only +- ✅ **GitHub Actions pinned** — Uses major version pins (@v4, @v1) for supply chain safety +- ✅ **No arbitrary code execution** — Workflow runs only controlled .NET build/test commands +- ✅ **Proper test isolation** — Separate result directories prevent path traversal +- ✅ **CI environment guard** — `CI=true` disables Tailwind in CI (line 43) + +**PR #6 Security Assessment (Template Cleanup):** + +Files changed: Counter.razor, Weather.razor deleted; RazorSmokeTests.cs modified + +Security findings: +- ✅ **Reduced attack surface** — Removed unused routes `/counter`, `/weather` +- ✅ **No authorization bypass** — Deleted components had no auth requirements +- ✅ **Test coverage maintained** — 91.64% line coverage, 74 tests passing +- ✅ **No secrets exposed** — All changes are code deletions only + +**PR #7 Security Assessment (Copyright Headers):** + +Files changed: 48 C# source files across all projects + +Security findings: +- ✅ **Zero functional changes** — Copyright headers are purely cosmetic comments +- ✅ **No secrets or credentials** — No password/key/token keywords found in diffs +- ✅ **Build verification** — All 74 tests passing, 0 errors, 0 warnings +- ✅ **CI checks passing** — build-and-test: SUCCESS (1m16s), Test Results: SUCCESS + +**Key Learnings:** + +1. **CI/CD Pipeline Security Checklist:** + - Verify GitHub Actions permissions follow least-privilege principle + - Check for secrets in workflow files or environment variables + - Ensure Actions pinned to major versions (not `@latest` or SHA) + - Review arbitrary code execution risks in workflow steps + - Validate test isolation (no shared directories) + +2. **Attack Surface Reduction Pattern:** + - Removing unused routes/components reduces potential entry points + - Ensure deletions don't break dependent code (test coverage crucial) + - Verify no authorization logic bypassed by removals + +3. **Copyright Header Review:** + - Non-functional changes (comments) still require security review + - Check for accidental secrets in diff hunks (grep for keywords) + - Verify CI passes before merge (headers shouldn't break build) + - Fast rebase workflow: conflicts auto-resolved when files deleted + +4. **Post-Merge Validation Process:** + - Always sync main after merge: `git checkout main && git pull` + - Build verification: `dotnet build src/Web/Web.csproj --configuration Release` + - Test verification: `dotnet test --no-restore` + - Coverage baseline: maintain 91%+ line coverage + +5. **Git Rebase for Conflict Resolution:** + - When PR conflicts with main (e.g., files deleted), use rebase: `git rebase origin/main` + - Git auto-drops duplicate commits (e.g., PR #7 lost 4 commits already in main) + - Force-push after rebase: `git push --force-with-lease` to update remote + - CI re-runs after force-push, ensuring rebased code tested + +**Decision Records Created:** +- `.squad/decisions/inbox/gandalf-pr5-pr6-merged.md` (PR #5 & #6) + +**Build Workaround:** +- `.slnx` solution build fails with CLR error 0x80131506 (unrelated to PRs) +- Use individual project builds: `dotnet build src/Web/Web.csproj` + ### PR #2 Security Audit — 2025-07 (squad/coverage-test-hardening-main) **Reviewed files:** RoleClaimsHelper.cs, ManageRoles.razor, Profile.razor, Program.cs, AssemblyInfo.cs, TestAuthorizationService.cs, RoleClaimsHelperTests.cs, NavMenu.razor, MainLayout.razor, Home.razor diff --git a/.squad/decisions.md b/.squad/decisions.md index a251bcee..4162312d 100644 --- a/.squad/decisions.md +++ b/.squad/decisions.md @@ -103,6 +103,83 @@ Adopted standardized 7-line copyright header format for all C# (`.cs`) files in - 9 additional lines per file (header + blank line separator) - Requires maintenance for new files (can be automated) +--- + +### 3. CI Workflow for Automated PR Validation + +**Status:** ✅ Implemented & Merged +**PR:** #5 +**Date:** 2026-04-18 +**Reviewer:** Gandalf (Security Officer) + +**Decision:** Approve and merge `.github/workflows/ci.yml` for PR validation pipeline. + +**What Changed:** +- Added `.github/workflows/ci.yml` — full CI pipeline for PR validation +- Workflow triggers on pull_request to main/squad/** and push to main +- Executes: build (Release) + Architecture/Unit/Integration tests + coverage reporting +- Uses GitHub Actions: checkout@v4, setup-dotnet@v4, cache@v4, test-reporter@v1, upload-artifact@v4, CodeCoverageSummary@v1.3.0, sticky-pull-request-comment@v2 + +**Security Assessment:** +1. ✅ **No hardcoded secrets** — workflow is clean, no credentials in source +2. ✅ **Least-privilege permissions** — `contents:read`, `checks:write`, `pull-requests:write` (minimal) +3. ✅ **Action pinning** — All actions pinned to major versions (@v4, @v1) from trusted publishers (GitHub, dorny, irongut, marocchino) +4. ✅ **No arbitrary code execution** — All commands are static, no eval of user input +5. ✅ **CI environment guard** — Sets `CI=true` to skip Tailwind compilation (appropriate) +6. ✅ **Test isolation** — Separate result directories per suite prevent cross-contamination + +**Verification:** +- ✅ Build succeeded (Release config) +- ✅ All 74 tests passing (Arch 6, Unit 59, Integration 9) +- ✅ Code coverage: 91.64% +- ✅ CI checks passed (build-and-test: SUCCESS, Test Results: SUCCESS) + +**Impact:** +- Automated validation now active on all future PRs to main and squad/** branches +- Coverage reporting added to PR comments +- Reduced manual security/build review overhead +- Enables coverage tracking and enforcement + +**Recommendations for Future PRs:** +1. All PRs to main or squad/** will trigger automated build + test validation +2. PR comments will show code coverage summaries; maintain ≥91% +3. PRs must pass CI checks before merge + +--- + +### 4. Template Cleanup Decision (Gandalf Security Review) + +**Status:** ✅ Implemented & Merged +**PR:** #6 +**Date:** 2026-04-18 +**Reviewer:** Gandalf (Security Officer) + +**Decision:** Approve and merge removal of unused demo pages. + +**What Changed:** +- Deleted `src/Web/Components/Pages/Counter.razor` (19 lines) +- Deleted `src/Web/Components/Pages/Weather.razor` (66 lines) +- Removed 2 obsolete test methods from `tests/Unit.Tests/Components/RazorSmokeTests.cs` (28 lines) +- Regenerated `src/Web/wwwroot/css/tailwind.css` (minimal diff) +- Total lines removed: 113 + +**Security Findings:** +1. ✅ **Reduced attack surface** — Removing unused routes (`/counter`, `/weather`) reduces potential attack vectors +2. ✅ **No authorization bypass** — Neither deleted component had `[Authorize]` attributes or role requirements +3. ✅ **Test coverage maintained** — 91.64% line coverage after removing obsolete tests +4. ✅ **No secrets exposed** — No configuration changes, no secret additions or removals + +**Verification:** +- ✅ Build succeeded (Release config, 0 errors, 0 warnings) +- ✅ All 74 tests passing (Arch 6, Unit 59, Integration 9) +- ✅ Code coverage: 91.64% maintained + +**Impact:** +- Cleaner codebase focused on blog functionality +- Reduced complexity and maintenance burden +- Smaller attack surface +- No security regressions introduced + ## Governance - All meaningful changes require team consensus