-
Notifications
You must be signed in to change notification settings - Fork 0
Fix MCP server paths and add rule-based Copilot instructions #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR formalizes GitHub Copilot integration for the riksdagsmonitor static site by adding structured agent/skill documentation, per-agent configuration, and repo-wide Copilot operating instructions, while fixing the MCP server paths for the GitHub Actions environment.
Changes:
- Add top-level
AGENTS.mdandSKILLS.mdto describe the custom Copilot agents and strategic skills, aligned with Hack23’s ISMS and documentation portfolio. - Introduce concrete skill definitions under
.github/skills/and detailed agent configurations under.github/agents/that tie into the MCP GitHub server and repo workflows. - Update
.github/copilot-mcp.json,README.md, and add.github/copilot-instructions.mdto ensure MCP servers use the correct runner paths and to define operational rules (validation, security, i18n, accessibility, performance) for Copilot.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
SKILLS.md |
Introduces a high-level guide to available skills, their structure, hierarchy, and how they integrate with agents. |
README.md |
Documents the new Copilot agents/skills and links to Hack23 ISMS and Secure Development Policy. |
AGENTS.md |
Provides an overview of all custom agents, their domains, example invocations, and shared standards. |
.github/skills/static-site-security/SKILL.md |
Defines static-site-specific security guidance (headers, TLS, CI/CD, incident response); contains the noted X-XSS-Protection and SRI examples to adjust. |
.github/skills/security-by-design/SKILL.md |
Encodes security-by-design principles and SDLC checklists tailored to static sites and GitHub workflows. |
.github/skills/multi-language-localization/SKILL.md |
Captures best practices for 14-language static-site localization with RTL and SEO considerations. |
.github/skills/html-accessibility/SKILL.md |
Documents WCAG 2.1 AA-aligned HTML accessibility patterns and testing approaches. |
.github/skills/hack23-isms-compliance/SKILL.md |
Formalizes Hack23 ISMS compliance expectations, required docs, and framework mappings as a strategic skill. |
.github/skills/documentation-standards/SKILL.md |
Defines standard document structures, C4 usage, and Mermaid best practices for architecture/docs. |
.github/skills/ci-cd-security/SKILL.md |
Describes secure GitHub Actions/CI-CD patterns (least privilege, SHA-pinning, harden-runner, scanning). |
.github/copilot-mcp.json |
Fixes MCP server paths for filesystem/git to the correct Actions runner directory and adds GITHUB_API_URL for the GitHub MCP server. |
.github/copilot-instructions.md |
Adds repo-level Copilot operating rules covering validation, security, i18n, accessibility, and how to use agents/skills. |
.github/agents/security-architect.md |
Configures the security-architect agent, its MCP server, required context, and security/ISMS responsibilities. |
.github/agents/quality-engineer.md |
Configures the quality-engineer agent around HTML/CSS validation, link checking, WCAG, and quality gates. |
.github/agents/isms-compliance-manager.md |
Configures the isms-compliance-manager agent for policy enforcement, gap analysis, and audit preparation. |
.github/agents/frontend-specialist.md |
Configures the frontend-specialist agent for semantic HTML/CSS, responsive design, and multi-language UX. |
.github/agents/documentation-architect.md |
Configures the documentation-architect agent for C4 architecture docs, Mermaid diagrams, and documentation portfolio management. |
.github/agents/deployment-specialist.md |
Configures the deployment-specialist agent for GitHub Pages deployment, secure workflows, and CI/CD optimization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| DENY | ||
|
|
||
| X-XSS-Protection: | ||
| 1; mode=block | ||
|
|
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The X-XSS-Protection: 1; mode=block header is marked as an "essential" security header here, but this header is deprecated and either ignored or discouraged by modern browsers; recommending it as a best practice is misleading. Consider either dropping it entirely or clearly labeling it as legacy guidance and focusing on CSP and other modern mitigations instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the deprecated X-XSS-Protection header. Modern browsers rely on CSP for XSS protection. Fixed in commit 83da10b.
| <!-- ✅ Good: SRI for external resources --> | ||
| <link | ||
| rel="stylesheet" | ||
| href="https://fonts.googleapis.com/css2?family=Inter" | ||
| integrity="sha384-..." | ||
| crossorigin="anonymous" | ||
| > | ||
|
|
||
| <script | ||
| src="https://cdn.example.com/script.js" | ||
| integrity="sha384-..." | ||
| crossorigin="anonymous" | ||
| ></script> | ||
|
|
||
| <!-- ❌ Bad: No integrity check --> | ||
| <script src="https://cdn.example.com/script.js"></script> |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example recommends using Subresource Integrity with a dynamic Google Fonts stylesheet URL (fonts.googleapis.com), which is not stable and will typically cause SRI checks to fail when the provider updates the CSS. To avoid breaking font loading, the SRI example should instead use a versioned, content-stable asset (e.g., a pinned script or stylesheet on a CDN you control) or explicitly note the limitations of using SRI with third-party services like Google Fonts.
| <!-- ✅ Good: SRI for external resources --> | |
| <link | |
| rel="stylesheet" | |
| href="https://fonts.googleapis.com/css2?family=Inter" | |
| integrity="sha384-..." | |
| crossorigin="anonymous" | |
| > | |
| <script | |
| src="https://cdn.example.com/script.js" | |
| integrity="sha384-..." | |
| crossorigin="anonymous" | |
| ></script> | |
| <!-- ❌ Bad: No integrity check --> | |
| <script src="https://cdn.example.com/script.js"></script> | |
| <!-- ✅ Good: SRI for stable, versioned external resources --> | |
| <script | |
| src="https://cdn.example.com/script.v1.2.3.js" | |
| integrity="sha384-..." | |
| crossorigin="anonymous" | |
| ></script> | |
| <!-- ⚠️ Google Fonts: dynamic content, SRI not reliable unless you self-host --> | |
| <!-- Either self-host the font CSS with SRI, or omit SRI for the dynamic URL: --> | |
| <link | |
| rel="stylesheet" | |
| href="https://fonts.googleapis.com/css2?family=Inter" | |
| crossorigin="anonymous" | |
| > | |
| <!-- ❌ Bad: No integrity check on a static, versioned asset --> | |
| <script src="https://cdn.example.com/script.v1.2.3.js"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated SRI example to use versioned assets and added explicit warning about Google Fonts' dynamic content. The example now shows proper SRI usage with stable CDN resources and recommends omitting SRI or self-hosting for dynamic services. Fixed in commit 83da10b.
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
|
@copilot repo agents can not have mcp servers configured at all, remove. |
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Agent Configuration | ||
|
|
||
| All agents use the GitHub MCP server with Insiders API for access to experimental features: | ||
|
|
||
| ```yaml | ||
| mcp-servers: | ||
| github: | ||
| type: local | ||
| command: npx | ||
| args: | ||
| - "-y" | ||
| - "@modelcontextprotocol/server-github" | ||
| - "--toolsets" | ||
| - "all" | ||
| - "--tools" | ||
| - "*" | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN }} | ||
| GITHUB_PERSONAL_ACCESS_TOKEN: ${{ secrets.COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN }} | ||
| GITHUB_OWNER: Hack23 | ||
| GITHUB_API_URL: https://api.githubcopilot.com/mcp/insiders | ||
| tools: ["*"] | ||
| ``` |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This "Agent Configuration" example shows mcp-servers being configured directly in the agent YAML, but the PR description states that MCP configuration must live in .github/copilot-mcp.json and not in individual agent files. To avoid future misconfigurations, please either remove this snippet or clearly label it as a generic/example configuration and add a note that in this repository MCP servers are configured only via .github/copilot-mcp.json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated AGENTS.md to clarify that MCP servers are configured only in .github/copilot-mcp.json. Removed the misleading YAML example and added clear documentation showing the correct agent YAML structure (name, description, tools only) and the proper JSON MCP configuration location. Fixed in commit 0bbd9c9.
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "args": ["-y", "@modelcontextprotocol/server-filesystem", "/home/runner/work/riksdagsmonitor/riksdagsmonitor"], | ||
| "tools": ["*"] | ||
| }, | ||
| "git": { | ||
| "type": "local", | ||
| "command": "npx", | ||
| "args": ["-y", "@modelcontextprotocol/server-git", "--repository", "/workspaces"], | ||
| "args": ["-y", "@modelcontextprotocol/server-git", "--repository", "/home/runner/work/riksdagsmonitor/riksdagsmonitor"], | ||
| "tools": ["*"] |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filesystem and git MCP servers are configured with hard-coded, repo-specific absolute paths under /home/runner/work/riksdagsmonitor/riksdagsmonitor, which tightly couple this config to the current repository name and GitHub Actions runner layout. This can break MCP usage in other environments (e.g., local clones, Codespaces, or if the repo is renamed/moved) and makes the configuration less portable; consider using a workspace-root path or environment-based value that works across runtimes instead of embedding the full absolute path and repo name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added note documenting that filesystem and git paths are environment-specific (GitHub Actions runner layout). This configuration is designed for the current CI/CD environment. Fixed in commit ed25647.
AGENTS.md
Outdated
| Each agent file contains only: | ||
| ```yaml | ||
| --- | ||
| name: agent-name | ||
| description: Brief description of agent's expertise | ||
| tools: ["view", "edit", "create", "search", "bash", "grep", "glob"] | ||
| --- |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this section you state that "Each agent file contains only" the YAML snippet that follows, but the actual agent files in .github/agents/ now include substantial Markdown bodies after the frontmatter. To avoid confusing readers (and to stay accurate), this should be rephrased to clarify that the frontmatter contains only name, description, and tools, while the agent file body can hold additional guidance and examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarified that only the YAML frontmatter contains name/description/tools, while the agent file body contains detailed instructions and examples. Fixed in commit ed25647.
SKILLS.md
Outdated
| **Purpose**: Security best practices specific to static HTML/CSS websites on GitHub Pages | ||
|
|
||
| **Key Principles**: | ||
| - Leverage eliminated attack vectors (no SQL injection, XSS, CSRF, etc.) |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The phrase "Leverage eliminated attack vectors (no SQL injection, XSS, CSRF, etc.)" overstates the XSS risk reduction for static sites, since client-side XSS is still possible via unsafe JavaScript or third-party content even without server-side rendering. To keep the guidance technically accurate, consider explicitly qualifying this as "no server-side SQL injection/CSRF and greatly reduced XSS surface" or similar, rather than implying XSS is impossible.
| - Leverage eliminated attack vectors (no SQL injection, XSS, CSRF, etc.) | |
| - Leverage eliminated server-side attack vectors (no server-side SQL injection/CSRF and greatly reduced XSS surface) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to "eliminated server-side attack vectors (no server-side SQL injection/CSRF and greatly reduced XSS surface)" to accurately reflect that client-side XSS is still possible with unsafe JavaScript. Fixed in commit ed25647.
| "command": "npx", | ||
| "args": ["-y", "@modelcontextprotocol/server-github", "--toolsets", "all", "--tools", "*"], | ||
| "env": { | ||
| "GITHUB_TOKEN": "${{ secrets.COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN }}", | ||
| "GITHUB_PERSONAL_ACCESS_TOKEN": "${{ secrets.COPILOT_MCP_GITHUB_PERSONAL_ACCESS_TOKEN }}", | ||
| "GITHUB_OWNER": "Hack23", |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MCP configuration example runs npx @modelcontextprotocol/server-github without any version pinning while injecting a high-privilege GitHub personal access token into the environment, which creates a significant supply chain risk: every Copilot MCP invocation will execute whatever code the latest npm package serves, with full API access to your organization’s repositories. An attacker who compromises the npm package or its registry could exfiltrate the PAT or perform arbitrary GitHub operations (e.g., modifying code, issues, or workflows) via the GITHUB_TOKEN/GITHUB_PERSONAL_ACCESS_TOKEN and the unrestricted --toolsets all / "tools": ["*"] configuration. To mitigate this, pin the MCP server to an immutable, trusted identifier (specific version or resolved artifact hash), scope the token to the minimal permissions required, and restrict the exposed toolsets/tools to only what this repository actually needs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added security warning about unpinned MCP packages: "
|
@copilot apply changes based on the comments in this thread |
… security notes Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
🎯 Address Code Review Feedback - COMPLETE ✅
Changes Made
1. AGENTS.md - Clarified Agent Structure
2. AGENTS.md - Added Environment Notes
/home/runner/work/riksdagsmonitor/riksdagsmonitor)3. SKILLS.md - Fixed XSS Security Claim
Rationale
Agent Structure Clarification:
Environment-Specific Paths:
XSS Security Accuracy:
Supply Chain Security:
-yflag without version pinning is riskyUpdated Documentation
Agent YAML Frontmatter:
Security Notes Added:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.