Add automated template submission pipeline for trusted publishers#785
Add automated template submission pipeline for trusted publishers#785jongio wants to merge 4 commits intoAzure:mainfrom
Conversation
jongio
requested review from
JeffreyCA,
hemarina,
rajeshkamal5050,
vhvb1989,
wbreza and
weikanglim
as code owners
There was a problem hiding this comment.
Pull request overview
This PR introduces GitHub-issue-driven automation for adding azd templates to the gallery (validation → templates.json update → PR creation), with an auto-merge path for allowlisted publishers. In addition, it includes a broad website redesign (new design tokens, gallery/filters UX updates, getting-started refresh), new service landing pages, and new Playwright e2e scaffolding.
Changes:
- Add template submission automation via issue form + workflow + repo validation script, plus trusted-publisher auto-merge workflow.
- Implement a site-wide visual refresh (design tokens, updated gallery hero/search/filters/cards, updated getting-started content).
- Add service landing page framework + 2 example service pages, and introduce Playwright configuration + initial e2e specs.
Reviewed changes
Copilot reviewed 48 out of 49 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
.github/ISSUE_TEMPLATE/template-submission.yml |
Adds an issue form used to collect template submission metadata. |
.github/trusted-publishers.json |
Adds an allowlist of trusted GitHub usernames for expedited handling. |
.github/workflows/template-submission.yml |
Adds issue/dispatch workflow to parse submissions, validate, update templates.json, and open a PR. |
.github/workflows/template-auto-merge.yml |
Adds auto-approval/auto-merge workflow for trusted publisher submissions. |
website/scripts/validate-template.js |
Adds a Node script to validate a GitHub repo URL via GitHub API and generate a UUID. |
website/docs/contribute.md |
Documents the new automated template submission path. |
website/package.json |
Adds Playwright test dependency. |
website/package-lock.json |
Locks Playwright (and related) dependency tree changes. |
website/playwright.config.ts |
Adds Playwright runner configuration for e2e tests. |
website/e2e/navigation-a11y.spec.ts |
Adds navigation + basic accessibility smoke tests. |
website/e2e/homepage.spec.ts |
Adds homepage e2e assertions for hero/search/cards/navbar. |
website/e2e/getting-started.spec.ts |
Adds getting-started page e2e assertions. |
website/e2e/gallery-filters.spec.ts |
Adds gallery filtering and empty-state e2e coverage. |
website/docusaurus.config.js |
Adds Google Fonts, adds “Services” dropdown, and updates primary CTA label. |
website/src/css/custom.css |
Introduces new design tokens (palette/type/spacing) and global styling adjustments. |
website/src/pages/styles.module.css |
Updates homepage/gallery page layout/styling (hero/search/filter/card spacing & colors). |
website/src/components/gallery/ShowcaseTemplateSearch/index.tsx |
Refreshes gallery hero content, adds stats bar and a templates CTA, improves search accessibility. |
website/src/components/gallery/ShowcaseTemplateSearch/styles.module.css |
Adds new hero/search CTA/stats styling for the gallery. |
website/src/components/gallery/ShowcaseLeftFilters/index.tsx |
Adds per-tag counts, improves keyboard accessibility for “Clear all”, and updates chevron images. |
website/src/components/gallery/ShowcaseLeftFilters/styles.module.css |
Updates filter styling and adds count badge styling. |
website/src/components/gallery/ShowcaseTagSelect/index.tsx |
Adds optional count display next to filter labels. |
website/src/components/gallery/ShowcaseEmptyResult/index.tsx |
Updates empty-state messaging and adds image width attribute. |
website/src/components/gallery/ShowcaseEmptyResult/styles.module.css |
Updates empty-state link color to the new primary token. |
website/src/components/gallery/ShowcaseCard/index.tsx |
Adds image sizing/aria labels and tweaks title interaction semantics. |
website/src/components/gallery/ShowcaseCard/styles.module.css |
Redesigns template cards (surface/border/shadow/type/inputs). |
website/src/components/gallery/ShowcaseCardPanel/index.tsx |
Improves button semantics/aria-labels and sets icon image dimensions. |
website/src/components/gallery/ShowcaseCardPanel/styles.module.css |
Updates card panel accent color to new primary. |
website/src/components/gallery/ShowcaseExtensionCard/styles.module.css |
Aligns extension card styling with new tokens (radius/colors/fonts). |
website/src/components/gallery/ShowcaseSurveyCard/styles.module.css |
Updates survey card/button styling to new tokens. |
website/src/components/gallery/ShowcaseMultipleAuthors/index.tsx |
Replaces var with const for split author arrays. |
website/src/components/gallery/ShowcaseMultipleAuthors/styles.module.css |
Updates author link colors to new primary token. |
website/src/components/styles.module.css |
Updates dark theme card/content colors. |
website/src/pages/ShowcaseCardPage.tsx |
Updates submission link color token. |
website/src/pages/getting-started/index.js |
Updates getting-started hero copy, adds 3-step section, and tweaks video iframe attributes/title. |
website/src/pages/getting-started/styles.module.css |
Redesigns getting-started layout and adds styles for steps + featured section. |
website/src/components/ServiceLandingPage/index.tsx |
Adds a reusable service landing page component that filters templates by service tag. |
website/src/components/ServiceLandingPage/styles.module.css |
Adds styles for service landing pages (hero, quickstart, templates, resources). |
website/src/pages/services/container-apps.tsx |
Adds Azure Container Apps service landing page config. |
website/src/pages/services/azure-functions.tsx |
Adds Azure Functions service landing page config. |
website/src/components/Feature.js |
Updates feature link color styling to match new palette. |
docs/spec.md |
Adds a redesign product spec document. |
baseline/sitemap.json |
Adds baseline sitemap artifact. |
baseline/report.md |
Adds baseline site report artifact. |
baseline/design-system.md |
Adds baseline design system artifact. |
baseline/design-inventory.json |
Adds baseline design inventory artifact. |
baseline/content-architecture.md |
Adds baseline information architecture artifact. |
.ai-team/team.md |
Adds AI team/roles documentation for redesign work. |
.ai-team/routing.md |
Adds routing rules documentation for redesign work. |
.ai-team/decisions.md |
Adds decisions log documentation for redesign work. |
Files not reviewed (1)
- website/package-lock.json: Language not supported
jongio
force-pushed
the
template-submission-automation
branch
from
cf2e33c to
740dd96
Compare
Addressing Copilot Review FeedbackThread 1 — Scope mismatch (.github/workflows/template-submission.yml): This was a false positive from the initial push before rebasing. The PR now contains only the 6 template submission automation files — no redesign or unrelated changes. The reviewer saw the old commit (cf2e33c) which included local commits that have since been cleaned up via rebase. Thread 2 — HTTP vs HTTPS validation ( Thread 3 — Auto-merge checks wrong identity ( |
| // Validate it is a GitHub URL | ||
| if ( | ||
| parsed.hostname !== "github.com" && | ||
| parsed.hostname !== "www.github.com" | ||
| ) { |
There was a problem hiding this comment.
The script accepts www.github.com but returns sourceUrl unchanged. Since the workflow stores this value, equivalent URLs can bypass dedupe (e.g., www.github.com/... vs github.com/..., .git suffix). Consider returning a canonicalized repo-root URL (normalized host, no .git, no trailing slash).
| const prBody = context.payload.pull_request.body || ''; | ||
| const issueMatch = prBody.match(/from issue #(\d+)/); | ||
| if (!issueMatch) { | ||
| core.setFailed('Could not determine originating issue from PR body'); |
There was a problem hiding this comment.
Verify trusted publisher fails if it can't parse from issue #NNN from the PR body. PRs created via workflow_dispatch intentionally omit the issue reference, but template-submission.yml can still add the trusted-publisher label—leading to auto-merge failures for dispatch-created PRs. Either avoid adding trusted-publisher for workflow_dispatch, or include a verifiable origin identifier in the PR body.
| core.setFailed('Could not determine originating issue from PR body'); | |
| core.info('No originating issue reference found in PR body; skipping trusted publisher verification for this PR.'); |
website/scripts/validate-template.js
Outdated
| // Extract owner/repo from path | ||
| const pathParts = parsed.pathname.replace(/^\//, "").replace(/\/$/, "").replace(/\.git$/, "").split("/"); | ||
| if (pathParts.length < 2 || !pathParts[0] || !pathParts[1]) { | ||
| errors.push( |
There was a problem hiding this comment.
Owner/repo extraction accepts URLs with extra path segments (any path with >=2 segments), even though the error message says the expected format is https://github.com/{owner}/{repo}. This can lead to storing non-canonical source URLs and makes deduping harder. Consider rejecting paths with extra segments and/or canonicalizing the stored URL to the repo root.
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Verify trusted publisher | ||
| id: verify | ||
| uses: actions/github-script@v7 |
There was a problem hiding this comment.
This workflow pins older action majors (actions/checkout@v4, actions/github-script@v7) while the repo’s other workflows use checkout v6 and github-script v8. Aligning versions across workflows reduces maintenance burden and keeps security/bugfix updates consistent.
| uses: actions/checkout@v4 | |
| - name: Verify trusted publisher | |
| id: verify | |
| uses: actions/github-script@v7 | |
| uses: actions/checkout@v6 | |
| - name: Verify trusted publisher | |
| id: verify | |
| uses: actions/github-script@v8 |
| const additionalTags = parseCsvTags(process.env.TPL_ADDITIONAL_TAGS); | ||
| for (const tag of additionalTags) { |
There was a problem hiding this comment.
additional_tags can include special admin tags like aicollection, but the workflow doesn't validate whether the template is actually in the curated AI collection (per the tag description in website/src/data/tags.tsx). Since trusted publisher PRs can be auto-merged, consider blocking aicollection (and similar admin-only tags) from additional_tags unless you add deterministic validation.
| const additionalTags = parseCsvTags(process.env.TPL_ADDITIONAL_TAGS); | |
| for (const tag of additionalTags) { | |
| const additionalTags = parseCsvTags(process.env.TPL_ADDITIONAL_TAGS); | |
| // Certain admin-only tags (e.g., "aicollection") must not be set via additional_tags. | |
| const disallowedAdditionalTags = new Set(['aicollection']); | |
| for (const tag of additionalTags) { | |
| if (disallowedAdditionalTags.has(tag)) { | |
| core.info(`Ignoring disallowed admin tag in additional_tags: "${tag}"`); | |
| continue; | |
| } |
| node website/scripts/validate-template.js "$SOURCE_REPO" > validation_result.json 2>validation_errors.log | ||
| if [ $? -ne 0 ]; then | ||
| echo "valid=false" >> $GITHUB_OUTPUT | ||
| { | ||
| echo 'errors<<EOF' | ||
| cat validation_result.json validation_errors.log | ||
| echo 'EOF' | ||
| } >> $GITHUB_OUTPUT | ||
| else | ||
| echo "valid=true" >> $GITHUB_OUTPUT | ||
| { | ||
| echo 'result<<EOF' | ||
| cat validation_result.json |
There was a problem hiding this comment.
This run: block assumes it can inspect $? after node ..., but GitHub Actions runs bash with -e by default; when node exits non-zero the step terminates immediately and these outputs won't be written. Wrap the node call in an if node ...; then ... else ... fi (or disable -e around the call) so validation failures can be reported via outputs/comments.
| node website/scripts/validate-template.js "$SOURCE_REPO" > validation_result.json 2>validation_errors.log | |
| if [ $? -ne 0 ]; then | |
| echo "valid=false" >> $GITHUB_OUTPUT | |
| { | |
| echo 'errors<<EOF' | |
| cat validation_result.json validation_errors.log | |
| echo 'EOF' | |
| } >> $GITHUB_OUTPUT | |
| else | |
| echo "valid=true" >> $GITHUB_OUTPUT | |
| { | |
| echo 'result<<EOF' | |
| cat validation_result.json | |
| if node website/scripts/validate-template.js "$SOURCE_REPO" > validation_result.json 2>validation_errors.log; then | |
| echo "valid=true" >> $GITHUB_OUTPUT | |
| { | |
| echo 'result<<EOF' | |
| cat validation_result.json | |
| echo 'EOF' | |
| } >> $GITHUB_OUTPUT | |
| else | |
| echo "valid=false" >> $GITHUB_OUTPUT | |
| { | |
| echo 'errors<<EOF' | |
| cat validation_result.json validation_errors.log |
| if (!['http:', 'https:'].includes(u.protocol)) { | ||
| throw new Error(`unsafe protocol "${u.protocol}"`); |
There was a problem hiding this comment.
validateUrl() allows http: URLs (['http:', 'https:']). Since the site is served over HTTPS, http: preview images can cause mixed-content blocking in browsers, and allowing non-HTTPS URLs weakens the security posture described in the PR. Consider requiring https: for user-supplied URLs here.
| if (!['http:', 'https:'].includes(u.protocol)) { | |
| throw new Error(`unsafe protocol "${u.protocol}"`); | |
| if (u.protocol !== 'https:') { | |
| throw new Error(`unsafe protocol "${u.protocol}" (only "https:" is allowed)`); |
| // Check for duplicate source URLs | ||
| const normalizedSource = sourceRepo.replace(/\/+$/, '').toLowerCase(); | ||
| const duplicate = templates.find( | ||
| t => t.source && t.source.replace(/\/+$/, '').toLowerCase() === normalizedSource |
There was a problem hiding this comment.
Duplicate detection only normalizes trailing slashes + lowercase. This misses common equivalent forms like a .git suffix or www.github.com host, so the workflow can add duplicate entries for the same repo. Consider canonicalizing the source URL (strip .git, normalize host to github.com, and possibly rewrite to https://github.com/{owner}/{repo}) before comparing/storing.
| // Check for duplicate source URLs | |
| const normalizedSource = sourceRepo.replace(/\/+$/, '').toLowerCase(); | |
| const duplicate = templates.find( | |
| t => t.source && t.source.replace(/\/+$/, '').toLowerCase() === normalizedSource | |
| /** | |
| * Canonicalize a repository source URL for duplicate detection. | |
| * - Trims whitespace | |
| * - Removes trailing slashes | |
| * - Strips a trailing `.git` | |
| * - Lowercases the result | |
| * - For GitHub URLs, normalizes host to github.com and rewrites to | |
| * https://github.com/{owner}/{repo} | |
| */ | |
| function canonicalizeSourceUrl(input) { | |
| if (!input) { | |
| return ''; | |
| } | |
| const trimmed = input.trim(); | |
| let urlObj; | |
| try { | |
| urlObj = new URL(trimmed); | |
| } catch { | |
| // If it's not a valid URL, fall back to basic normalization | |
| return trimmed | |
| .replace(/\/+$/, '') | |
| .replace(/\.git$/i, '') | |
| .toLowerCase(); | |
| } | |
| const hostname = urlObj.hostname.toLowerCase(); | |
| if (hostname === 'github.com' || hostname === 'www.github.com') { | |
| const parts = urlObj.pathname.replace(/\/+$/, '').split('/').filter(Boolean); | |
| if (parts.length >= 2) { | |
| const owner = parts[0]; | |
| const repo = parts[1].replace(/\.git$/i, ''); | |
| return `https://github.com/${owner}/${repo}`.toLowerCase(); | |
| } | |
| } | |
| // Non-GitHub or unrecognized GitHub path: basic normalization of full URL | |
| let href = urlObj.href; | |
| href = href.replace(/\/+$/, '').replace(/\.git$/i, ''); | |
| return href.toLowerCase(); | |
| } | |
| // Check for duplicate source URLs | |
| const canonicalSource = canonicalizeSourceUrl(sourceRepo); | |
| const duplicate = templates.find( | |
| t => t.source && canonicalizeSourceUrl(t.source) === canonicalSource |
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| gh pr review ${{ github.event.pull_request.number }} --approve --body "Auto-approved: PR author is a trusted publisher." |
There was a problem hiding this comment.
The approval review body says "PR author is a trusted publisher", but this workflow actually verifies the originating issue submitter (not the PR author, which is often github-actions[bot]). Update the message to reflect what was verified so the audit trail is accurate.
| gh pr review ${{ github.event.pull_request.number }} --approve --body "Auto-approved: PR author is a trusted publisher." | |
| gh pr review ${{ github.event.pull_request.number }} --approve --body "Auto-approved: originating issue submitter is a trusted publisher." |
|
|
||
| - name: Parse issue body | ||
| id: parse | ||
| uses: actions/github-script@v7 |
There was a problem hiding this comment.
This workflow uses older action versions (actions/github-script@v7, peter-evans/create-pull-request@v7) while the repo uses newer major versions elsewhere (e.g., github-script v8 and create-pull-request v8 in .github/workflows/extension-submission.yml). Consider updating to match the repo’s pinned majors for consistency and security fixes.
| uses: actions/github-script@v7 | |
| uses: actions/github-script@v8 |
- GitHub Issue form for template submissions with structured fields - Automated validation workflow: URL checks, HTTPS-only, duplicate detection - Auto-merge workflow for trusted publisher submissions - Trusted publishers registry (trusted-publishers.json) - Template validation script with URL canonicalization - Updated contribution docs with new submission process
jongio
force-pushed
the
template-submission-automation
branch
from
e87c3df to
a626727
Compare
hemarina
left a comment
hemarina
left a comment
There was a problem hiding this comment.
Code Review: PR #785 — Add automated template submission pipeline for trusted publishers
Great work building out this automation pipeline — the issue form design is solid, the SSRF protections in validate-template.js are well-thought-out, and the prior review feedback (HTTP protocol, PR-author verification) was addressed promptly. Below are additional findings that supplement the existing Copilot review.
Summary
| Priority | Count |
|---|---|
| Critical | 2 |
| High | 6 |
| Medium | 4 |
| Total | 12 |
Overall Assessment: Request changes
What Looks Good
- SSRF protection in
validate-template.jsis well-implemented — HTTPS-only, GitHub hostname validation,encodeURIComponent, AbortController timeout. - Issue form design is thorough — clear descriptions, constrained dropdowns, appropriate required/optional flags.
- Prior review feedback was addressed — HTTP protocol and PR-author bugs fixed in
e87c3df. - Security model is version-controlled — trusted-publishers.json changes require PR review.
See inline comments for all 12 findings.
| } | ||
|
|
||
| const issueNumber = parseInt(issueMatch[1], 10); | ||
| const { data: issue } = await github.rest.issues.get({ |
There was a problem hiding this comment.
[Critical] PR body forgery enables trusted-publisher impersonation
The auto-merge workflow extracts the originating issue number from the PR body via prBody.match(/from issue #(\d+)/), then checks if that issue's creator is a trusted publisher. It never verifies who created the PR or that the PR was generated by the template-submission workflow. A collaborator could create a PR with arbitrary changes, reference a trusted publisher's issue in the body text, and have the trusted-publisher label applied — triggering auto-approval and merge.
Impact: Complete bypass of the trusted-publisher security model.
Suggested Fix: Add checks that verify PR author is github-actions[bot] AND that the PR only modifies website/static/templates.json.
| github.event_name == 'workflow_dispatch' || | ||
| contains(github.event.issue.labels.*.name, 'template-submission') | ||
| runs-on: ubuntu-latest | ||
| steps: |
There was a problem hiding this comment.
[Critical] No concurrency control — lost-update race on templates.json
The workflow performs a non-atomic read-modify-write on templates.json. If two issues are labeled simultaneously, both runs checkout the same base, both append only their entry, and both create PRs. Merging the first makes the second's base stale — merging it silently drops the first entry with no merge conflict.
Impact: Template submissions can be silently lost.
Suggested Fix:
jobs:
process-template:
concurrency:
group: template-submission-mutex
cancel-in-progress: false| token: ${{ secrets.GITHUB_TOKEN }} | ||
| commit-message: "Add template: ${{ steps.parse.outputs.title }} from ${{ github.event_name == 'workflow_dispatch' && 'manual dispatch' || format('issue #{0}', github.event.issue.number) }}" | ||
| branch: "template-submission-${{ github.event_name == 'workflow_dispatch' && github.run_id || github.event.issue.number }}" | ||
| title: "Add template: ${{ steps.parse.outputs.title }}" |
There was a problem hiding this comment.
[High] GITHUB_TOKEN events won't trigger auto-merge workflow
peter-evans/create-pull-request uses secrets.GITHUB_TOKEN. GitHub prevents events from GITHUB_TOKEN from triggering other workflows. The trusted-publisher label added during PR creation will not fire the pull_request: [labeled] event that auto-merge listens for — making the entire auto-merge flow non-functional as designed.
Suggested Fix: Document that auto-merge requires manual label re-application, or use a GitHub App token with scoped permissions (after implementing PR-author and file-scope safeguards).
| } | ||
|
|
||
| const config = JSON.parse(fs.readFileSync(trustedPath, 'utf8')); | ||
|
|
There was a problem hiding this comment.
[High] Unguarded JSON.parse on trusted-publishers.json risks opaque crash
JSON.parse(fs.readFileSync(trustedPath, 'utf8')) has no try/catch. Malformed JSON produces an unhandled SyntaxError with no diagnostic context. Same issue exists in the template-submission workflow for VALIDATION_RESULT and templates.json.
Suggested Fix: Wrap in try/catch with descriptive core.setFailed() messages.
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| gh pr checks ${{ github.event.pull_request.number }} --watch --fail-fast | ||
|
|
There was a problem hiding this comment.
[High] gh pr checks --watch has no timeout — can hang indefinitely
gh pr checks --watch --fail-fast blocks until all checks complete. A stuck external status check causes this step to hang until the 6-hour Actions job limit, consuming runner minutes with no output.
Suggested Fix: Add timeout-minutes: 30 to this step.
| repo: context.repo.repo, | ||
| issue_number: issueNumber, | ||
| }); | ||
| const submitter = issue.user.login; |
There was a problem hiding this comment.
[High] Unhandled API error from github.rest.issues.get()
No try-catch around this API call. Rate limits (403), server errors (500), or deleted issues throw unhandled Octokit exceptions with opaque stack traces.
Suggested Fix: Wrap in try/catch with core.setFailed() including issue number and error status.
| echo "valid=true" >> $GITHUB_OUTPUT | ||
| { | ||
| echo 'result<<EOF' | ||
| cat validation_result.json |
There was a problem hiding this comment.
[Medium] Heredoc EOF delimiter can be injected via user input
Validation errors use EOF as heredoc delimiter for GITHUB_OUTPUT. Crafted input producing stderr containing exactly EOF prematurely closes the heredoc and could inject additional output key-value pairs.
Suggested Fix: Use a random delimiter: DELIMITER=$(uuidgen).
| const azureServices = extractField(body, 'Azure Services'); | ||
| const additionalTags = extractField(body, 'Additional Tags'); | ||
|
|
||
| if (!sourceRepo || !title || !description || !author || !authorUrl) { |
There was a problem hiding this comment.
[Medium] Missing input validation for authorType and iacProvider
The required-fields guard checks !sourceRepo || !title || !description || !author || !authorUrl but omits authorType and iacProvider. API-created issues that bypass form validation can submit empty values, causing incorrect tags or toLowerCase() errors downstream.
Suggested Fix: Add || !authorType || !iacProvider to the guard.
| author: process.env.TPL_AUTHOR, | ||
| source: sourceRepo, | ||
| tags: tags, | ||
| id: validation.generatedId |
There was a problem hiding this comment.
[Medium] No guard on validation.generatedId
id: validation.generatedId is used without checking existence. A missing field silently writes id: undefined into templates.json, corrupting the gallery data.
Suggested Fix: Add: if (!validation.generatedId) { core.setFailed('Validation result missing generatedId'); return; }
| description: "Comma-separated extra tags (e.g., ai, mongodb, kubernetes)" | ||
| placeholder: "ai, mongodb" | ||
| validations: | ||
| required: false |
There was a problem hiding this comment.
[Medium] Consider adding auto-merge opt-out for trusted publishers
There are cases where a trusted publisher submits a template but wants to merge the PR at a later date (e.g., for a private repo not yet public). Currently there's no way to opt out of auto-merge per submission.
Suggested Fix: Add a dropdown to the issue form with options like 'Yes (default)' and 'No - I'll merge manually', then conditionally skip the trusted-publisher label in the submission workflow when opt-out is selected.
|
@kristenwomack @haileyhuber8 Do we want to allow auto-approve? This is a design choice. I think the current blocker of PR submission is not how often it is reviewed. It's more about if internal contributors provide test proof of templates works, or they go to manual testing process (which the process is longer). I'm OK with auto approve but is a bit worry if the workflow is too automatic. |
… coverage Review fixes (20 items from @hemarina + copilot-pull-request-reviewer): Security (Critical): - Prevent PR body forgery by verifying PR author is github-actions[bot] and branch matches template-submission-{N} pattern - Add concurrency control to prevent lost-update race on templates.json - Replace fixed EOF heredoc delimiter with dynamic one to prevent injection Security (High): - Wrap all JSON.parse/API calls in try/catch with descriptive errors - Add timeout-minutes: 30 to gh pr checks --watch step - Require HTTPS-only in inline validateUrl (was allowing HTTP) - Document GITHUB_TOKEN limitation for cross-workflow triggers Quality: - Add URL canonicalization (strip www., .git, trailing slash, extra segments) - Handle workflow_dispatch PRs gracefully in auto-merge - Update action versions to match repo standard (checkout@v6, github-script@v8) - Block admin-only tags (aicollection, popular) from automated submissions - Fix bash -e compatibility in validation step - Fix approval message to say issue submitter not PR author Testing: - Add 25 new tests for validate-template.js (URL canonicalization, SSRF protection, HTTPS enforcement, error cases) - Total: 40/40 tests pass across 3 suites Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Validate enum fields (Author Type, IaC Provider) against allowlists to prevent textarea header-injection overrides - Strip HTML from all free-text fields (title, description, author, tags) at both parse and update stages (defense-in-depth) - Sanitize error output in issue comments to prevent markdown breakout - Add field length limits (title: 200, description: 2000, author: 100) and tag limits (50 chars, max 20 per field) to prevent payload bloat - Cross-validate branch number against issue number in auto-merge to prevent issue-reference mismatch attacks - Strip HTML from individual CSV tag values Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addressing All Review FeedbackThanks @hemarina for the thorough review! All 20 findings (yours + Copilot re-review) have been addressed across two commits: Commit
|
| Category | Fixes |
|---|---|
| Security (Critical) | PR body forgery prevention (bot author + branch pattern check), concurrency control on templates.json, dynamic heredoc delimiter |
| Security (High) | try/catch on all JSON.parse/API calls, HTTPS-only in inline validateUrl, timeout on gh pr checks --watch, GITHUB_TOKEN limitation documented |
| Quality | URL canonicalization (www/.git/trailing slash/extra segments), workflow_dispatch graceful handling, action versions updated (v6/v8), admin-only tag blocking, bash -e fix, approval message corrected |
| Testing | 25 new tests for validate-template.js (40 total, 0 failures) |
Commit 514f2ed — Input validation hardening
| Fix | Detail |
|---|---|
| Enum validation | Author Type and IaC Provider validated against allowlists (prevents textarea header-injection) |
| HTML stripping | stripHtml() applied to title, description, author, and tags at both parse and update stages |
| Markdown sanitization | Error output sanitized before embedding in issue comments |
| Field length limits | title: 200, description: 2000, author: 100 chars; tags: 50 chars, max 20 per field |
| Branch↔Issue cross-validation | Auto-merge verifies branch number matches issue number in PR body |
Re: auto-approve concern
The auto-merge pipeline now has 4 defense layers:
- PR authorship: Must be
github-actions[bot]with branchtemplate-submission-{N} - Branch↔Issue cross-validation: Branch number must match referenced issue number
- Issue submitter verification: Original issue creator must be in
trusted-publishers.json - GITHUB_TOKEN gate: Label events from GITHUB_TOKEN don't trigger workflows — a maintainer must manually re-apply the label to activate auto-merge
In practice, auto-merge is human-gated (layer 4) and everything in templates.json is rollbackable.
Known architectural limitations (documented, not code-fixable)
- Action version tags are floating (SHA pinning recommended for
peter-evans/create-pull-request) - No rate limiting on submissions beyond GitHub defaults + concurrency serialization
The auto-merge design was over-engineered: - GITHUB_TOKEN events can't trigger other workflows, making auto-merge dead code - All security hardening (bot author check, branch cross-validation, trusted-publishers.json) existed solely to defend automated approval - A human still had to manually re-apply the label anyway - The extension submission pipeline has zero auto-merge and works fine Simplified approach: issue -> validate -> PR -> human approves. Done. Removed: - .github/workflows/template-auto-merge.yml (122 lines) - .github/trusted-publishers.json - Trusted publisher check step from template-submission.yml - GITHUB_TOKEN limitation comments and trusted-publisher label logic Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Closing in favor of a fresh, simplified implementation. The auto-merge/trusted-publisher complexity was over-engineered — a human approval click is all that's needed. New PR incoming with clean design. |
Summary
Mirrors the existing extension submission automation for templates. Trusted internal partners can now submit templates via a GitHub issue form instead of manually editing the 5500-line templates.json. The workflow validates the submission, generates the JSON entry, and creates a PR automatically. Trusted publishers get auto-merge after CI passes.
Closes #784
Components
.github/ISSUE_TEMPLATE/template-submission.ymlwebsite/scripts/validate-template.js.github/workflows/template-submission.yml.github/trusted-publishers.json.github/workflows/template-auto-merge.ymlwebsite/docs/contribute.mdHow It Works
For any contributor:
For trusted publishers (listed in trusted-publishers.json):
Security Model
Testing
npm test-- 15/15 tests pass (0 failures)npm run build-- builds successfully