ci: add markdown link checker in workflow #544#586
ci: add markdown link checker in workflow #544#586imran-siddique merged 3 commits intomicrosoft:mainfrom
Conversation
|
Welcome to the Agent Governance Toolkit! Thanks for your first pull request. |
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Review Feedback
This pull request introduces a GitHub Actions workflow to validate markdown links in the repository. While this is a maintenance-focused addition, it indirectly contributes to the project's security and reliability by ensuring documentation integrity. Below is the detailed review:
🔴 CRITICAL
No critical issues identified in this PR.
🟡 WARNING
- Backward Compatibility:
- The workflow triggers on
pull_requestevents for markdown files (**/*.md). If the repository contains markdown files that are dynamically generated or excluded from version control, this workflow might not account for them. Ensure this does not inadvertently affect contributors or CI/CD processes.
- The workflow triggers on
💡 SUGGESTION
-
Workflow Robustness:
- The
lycheeverse/lychee-actionis pinned to versionv1.9.0. While this is good practice, consider using a SHA-based pinning for additional security against supply chain attacks. For example:uses: lycheeverse/lychee-action@<SHA>
- Alternatively, you could configure Dependabot to monitor GitHub Actions dependencies for updates.
- The
-
Error Reporting:
- The
argsfield includes--verbose, which is helpful for debugging. However, consider adding a mechanism to summarize the errors (e.g., number of broken links) for better visibility in the CI logs.
- The
-
Fail Behavior:
- The
fail: trueconfiguration ensures the workflow fails if broken links are detected. This is appropriate for pull requests but might be overly strict forworkflow_dispatchruns. Consider adding a conditional flag to allow manual runs without failing the workflow.
- The
-
Documentation:
- Update the repository's
CONTRIBUTING.mdfile to inform contributors about this new CI check. This will help them proactively fix broken links before submitting pull requests.
- Update the repository's
-
Testing:
- While this workflow does not directly affect the Python packages, it would be prudent to test the workflow on a branch with intentionally broken links to ensure it behaves as expected.
-
Exclusion Rules:
- The
--exclude-loopbackflag excludes localhost links. If there are other types of links (e.g., internal links or links to private resources) that should be excluded, consider adding them explicitly to the configuration.
- The
-
Caching:
- To optimize performance, consider caching the results of the link checker for unchanged files across runs. This can reduce unnecessary checks and improve CI efficiency.
Summary
This PR is a solid addition to the CI/CD pipeline for improving documentation quality. Addressing the suggestions above will enhance the workflow's robustness and usability. No critical security or breaking changes were identified.
🤖 AI Agent: contributor-guide — Welcome! 🎉Welcome! 🎉Hi there, and welcome to the microsoft/agent-governance-toolkit community! Thank you so much for taking the time to contribute. It's always exciting to see new contributors, and we’re thrilled to have you here. 😊 What You Did Well 🌟First off, great job on:
Suggestions for Improvement ✨Here are a few suggestions to make your contribution even better:
Resources to Help You 📚Here are some resources that might be helpful as you continue contributing:
Next Steps 🚀
Thank you again for your contribution! If you have any questions or need help, don’t hesitate to ask. We’re here to support you. 😊 Looking forward to seeing your updates! 🚀 |
🤖 AI Agent: security-scanner — Security Analysis of PR #544Security Analysis of PR #544This PR introduces a GitHub Actions workflow to validate markdown links in the repository. While this is a maintenance change and does not directly impact the core functionality of the Findings1. Potential Credential Exposure in LogsRisk: 🔴 CRITICAL Attack Vector: Suggested Fix:
2. Supply Chain Risk: Dependency on External ActionsRisk: 🟠 HIGH
These dependencies could introduce a supply chain risk if the maintainers of these actions are compromised or if malicious code is introduced in future versions. Attack Vector: Suggested Fix:
3. Improper Input Handling for
|
| Finding | Risk | Suggested Fix |
|---|---|---|
| Credential exposure in logs | 🔴 CRITICAL | Remove --verbose flag or sanitize logs to redact sensitive information. |
| Supply chain risk from external actions | 🟠 HIGH | Pin dependencies to specific commit SHAs and audit third-party actions regularly. |
Improper input handling for lychee-action |
🟡 MEDIUM | Validate and sanitize all_changed_files output before passing to the action. |
| Lack of restriction on workflow execution | 🟡 MEDIUM | Use pull_request_target and restrict GitHub Actions token permissions. |
Final Recommendation
While this PR primarily introduces a CI/CD maintenance workflow, it has potential security implications due to the use of third-party actions, verbose logging, and input handling. Address the critical and high-risk findings before merging to ensure the security of the repository and its workflows.
|
@microsoft-github-policy-service agree |
|
@imran-siddique, I've addressed the markdown link issues in this PR. Could you please take a look and provide a manual review/approval when you have a moment? Thanks! |
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Review of PR #544: ci: add markdown link checker in workflow
Summary
This PR introduces a GitHub Actions workflow to validate markdown links in the repository using the lychee-action. The workflow is triggered on pull requests that modify .md files and can also be manually triggered via workflow_dispatch.
Feedback
🔴 CRITICAL: Security Concerns
- Untrusted External Links in CI/CD:
- The
lychee-actionchecks external links, which could potentially expose the CI/CD pipeline to malicious or compromised URLs. This could lead to:- Denial of Service (DoS) if the pipeline is overwhelmed by slow or malicious responses.
- Data leakage if sensitive information is inadvertently sent to external servers.
- Recommendation: Add a whitelist or domain filtering mechanism to restrict link validation to trusted domains. Alternatively, consider running the link checker in a sandboxed environment.
- The
💡 SUGGESTION: Workflow Optimization
-
Avoid Running on Unchanged Markdown Files:
- The
if: steps.changed-files.outputs.any_changed == 'true'condition ensures the workflow runs only if markdown files are changed. However, theargsparameter (${{ steps.changed-files.outputs.all_changed_files }}) passes all changed files, which might include non-markdown files due to misconfiguration. - Recommendation: Explicitly filter for
.mdfiles in theargsparameter to avoid unnecessary checks.
- The
-
Parallel Execution:
- For repositories with many markdown files, the link checker might take significant time to complete.
- Recommendation: Investigate whether
lychee-actionsupports parallel execution or batching to improve performance.
💡 SUGGESTION: Error Handling
- Graceful Failure:
- The
fail: trueconfiguration ensures the workflow fails if broken links are detected. While this is useful for enforcing link correctness, it might block PRs unnecessarily if external links are temporarily down. - Recommendation: Consider adding a retry mechanism or marking external link failures as warnings instead of errors.
- The
💡 SUGGESTION: Documentation
- Workflow Documentation:
- The PR does not include documentation for the new workflow. Contributors might not be aware of its existence or how to debug failures.
- Recommendation: Add a section in the repository's
CONTRIBUTING.mdorREADME.mdexplaining the markdown link checker workflow and how to address failures.
🔴 CRITICAL: OWASP Agentic Top 10 Compliance
- External Dependency Trust:
- The
lychee-actionis an external dependency. If this action is compromised, it could introduce vulnerabilities into the CI/CD pipeline. - Recommendation: Pin the action version (
@v1.9.0) to avoid unexpected updates. Regularly audit the action's repository for security advisories.
- The
🟡 WARNING: Backward Compatibility
- Impact on Existing PRs:
- This workflow introduces a new CI/CD check that might fail for existing PRs with broken links, potentially blocking their merge.
- Recommendation: Communicate this change to contributors and provide a grace period for fixing broken links in existing PRs.
Actionable Items
- 🔴 Add domain filtering or sandboxing to mitigate risks from untrusted external links.
- 💡 Explicitly filter
.mdfiles in theargsparameter to avoid unnecessary checks. - 💡 Investigate parallel execution or batching for performance improvements.
- 💡 Add retry mechanisms or mark external link failures as warnings.
- 💡 Document the workflow in
CONTRIBUTING.mdorREADME.md. - 🔴 Pin the
lychee-actionversion and audit its repository regularly. - 🟡 Communicate the new workflow to contributors and provide a grace period for fixing broken links.
Final Assessment
This PR introduces a useful maintenance feature but requires additional safeguards to ensure security and usability. Addressing the critical issues flagged above is essential before merging.
There was a problem hiding this comment.
🤖 AI Agent: code-reviewer
Feedback on PR #544: Adding Markdown Link Checker Workflow
🔴 CRITICAL
No critical issues identified in this PR. The changes are limited to CI/CD workflow updates and do not directly impact the security or functionality of the core packages.
🟡 WARNING
- Potential Breaking Change in CI/CD Workflow:
- The
lycheeverse/lychee-action@v1.9.0is used for link checking. If this action is deprecated or changes its behavior in future versions, it could break the CI/CD pipeline. Consider pinning the action to a specific commit SHA instead of a version tag (v1.9.0) to ensure stability.
- The
💡 SUGGESTIONS
-
Improve Workflow Robustness:
- The
argsfield in theRun Link Checkerstep directly passes the list of changed files (${{ steps.changed-files.outputs.all_changed_files }}). If the list of files is empty or improperly formatted, the link checker might fail. Consider adding a fallback mechanism or validation step to ensure the input is always valid.
- The
-
Exclude Known False Positives:
- The
--exclude-loopbackflag is used to ignore localhost links. You might also want to exclude other known false positives, such as internal links that are dynamically generated or links to external services that are temporarily unavailable. This can be configured using an exclusion list.
- The
-
Add Documentation for Workflow:
- Update the repository's
CONTRIBUTING.mdor equivalent documentation to explain the purpose of this workflow and how contributors can address link-checking failures. This will help new contributors understand the CI/CD process better.
- Update the repository's
-
Test Workflow on a Variety of Markdown Files:
- Ensure the workflow has been tested on Markdown files with different types of links (e.g., relative links, absolute links, anchors) to confirm its reliability.
-
Optimize Workflow Trigger:
- The workflow currently triggers on all Markdown file changes (
**/*.md). If the repository contains Markdown files that are not relevant to the project (e.g., archived documentation), consider refining thepathsfilter to exclude such files.
- The workflow currently triggers on all Markdown file changes (
-
Add a Timeout:
- To prevent the workflow from hanging indefinitely due to network issues or unresponsive links, consider adding a timeout to the
lychee-actionstep.
- To prevent the workflow from hanging indefinitely due to network issues or unresponsive links, consider adding a timeout to the
Summary
This PR introduces a useful maintenance feature to validate Markdown links in the repository. While the implementation is straightforward, there are opportunities to improve robustness and documentation. Addressing the above suggestions will ensure the workflow remains reliable and easy to use for contributors.
…emoval Audit of 14 community PRs merged in the last 7 days. Fixes: 1. MIT license headers added to 122 source files (99 .py, 19 .ts, 4 .sh) that were missing the required copyright notice 2. SHA-pin 4 unpinned GitHub Actions (from PRs microsoft#586, microsoft#552): - actions/checkout@v4 -> @11bd71901bbe5b1630ceea73d27597364c9af683 - tj-actions/changed-files@v46 -> @ed68ef82c095e0d48ec87eccea555d944a631a4c - lycheeverse/lychee-action@v1.9.0 -> @22134d37a1fff6c2974df9c92a7c7e1e86a08f9c 3. Replace 10 innerHTML usages with safe DOM APIs in Chrome extension (aws.ts, github.ts, jira.ts). 3 complex templates kept with SECURITY comments. 4. Verified: pickle.loads in security_skills.py is detection-only (safe). shell=True in secure_codegen.py is test data only (safe). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…emoval (#615) Audit of 14 community PRs merged in the last 7 days. Fixes: 1. MIT license headers added to 122 source files (99 .py, 19 .ts, 4 .sh) that were missing the required copyright notice 2. SHA-pin 4 unpinned GitHub Actions (from PRs #586, #552): - actions/checkout@v4 -> @11bd71901bbe5b1630ceea73d27597364c9af683 - tj-actions/changed-files@v46 -> @ed68ef82c095e0d48ec87eccea555d944a631a4c - lycheeverse/lychee-action@v1.9.0 -> @22134d37a1fff6c2974df9c92a7c7e1e86a08f9c 3. Replace 10 innerHTML usages with safe DOM APIs in Chrome extension (aws.ts, github.ts, jira.ts). 3 complex templates kept with SECURITY comments. 4. Verified: pickle.loads in security_skills.py is detection-only (safe). shell=True in secure_codegen.py is test data only (safe). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…emoval (#616) Audit of 14 community PRs merged in the last 7 days. Fixes: 1. MIT license headers added to 122 source files (99 .py, 19 .ts, 4 .sh) that were missing the required copyright notice 2. SHA-pin 4 unpinned GitHub Actions (from PRs #586, #552): - actions/checkout@v4 -> @11bd71901bbe5b1630ceea73d27597364c9af683 - tj-actions/changed-files@v46 -> @ed68ef82c095e0d48ec87eccea555d944a631a4c - lycheeverse/lychee-action@v1.9.0 -> @22134d37a1fff6c2974df9c92a7c7e1e86a08f9c 3. Replace 10 innerHTML usages with safe DOM APIs in Chrome extension (aws.ts, github.ts, jira.ts). 3 complex templates kept with SECURITY comments. 4. Verified: pickle.loads in security_skills.py is detection-only (safe). shell=True in secure_codegen.py is test data only (safe). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
This PR addresses issue #544 by adding an automated GitHub Action workflow to validate markdown links within the repository.
Type of Change
Package(s) Affected
Checklist
Related Issues
Fixes #544