Skip to content

Exclude 404.html from lychee link checks #1265

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.

Sign up for GitHub

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

Merged
sbryngelson merged 3 commits into from
Feb 25, 2026
Merged

Exclude 404.html from lychee link checks #1265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52d10ba
Exclude 404.html from lychee link checks
sbryngelson Feb 25, 2026
93b942b
Merge branch 'master' into fix/lychee-404-exclusion
sbryngelson Feb 25, 2026
f2464c9
Install unzip for bun setup in workflow
sbryngelson Feb 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 7 additions & 20 deletions .github/workflows/claude-code-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ jobs:
id-token: write

steps:
- name: Install unzip (required by bun setup)
run: |
sudo apt-get update
sudo apt-get install -y unzip

# IMPORTANT: checkout BASE repo only (safe on forks)
- name: Checkout base repo (safe)
uses: actions/checkout@v4
Expand All @@ -24,7 +29,6 @@ jobs:
- name: Run Claude Code Review
uses: anthropics/claude-code-action@v1
env:
# Ensure the action can execute the Claude CLI it installs.
PATH: /home/runner/.local/bin:${{ env.PATH }}
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
Expand All @@ -35,27 +39,10 @@ jobs:

claude_args: >
--dangerously-skip-permissions
--dangerouslyDisableSandbox
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the --dangerouslyDisableSandbox flag is a security concern. This flag disables the sandbox security mechanism that isolates code execution.

Before enabling this flag, you should:

  1. Document why it's necessary (what functionality is broken without it?)
  2. Assess the security implications in the context of pull_request_target which already has security considerations
  3. Consider alternative solutions that don't compromise the sandbox

This is particularly important since the workflow runs on pull_request_target, which has access to repository secrets and write permissions.

Suggested change
--dangerouslyDisableSandbox

Copilot uses AI. Check for mistakes.
--max-turns 80
--allowedTools
"Bash(gh pr view:*)"
"Bash(gh pr diff:*)"
"Bash(gh pr comment:*)"
"Bash(gh api:*)"
"Bash(cat:*)"
"Bash(ls:*)"
"Bash(find:*)"
"Bash(grep:*)"
"Bash(sed:*)"
"Bash(awk:*)"
"Bash(head:*)"
"Bash(tail:*)"
"Bash(wc:*)"
"Bash(sort:*)"
"Bash(uniq:*)"
"Bash(cut:*)"
"Bash(xargs:*)"
"Bash(jq:*)"
"Bash(python3:*)"
"Bash"
Comment on lines 18 to +45
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR description states it's about excluding 404.html from lychee link checks to fix a CI issue. However, the changes to .github/workflows/claude-code-review.yml are unrelated to the stated purpose:

  1. Adding unzip installation step
  2. Removing a PATH comment
  3. Adding --dangerouslyDisableSandbox flag
  4. Simplifying allowedTools from specific command patterns to just "Bash"

These changes to the Claude Code Review workflow appear unrelated to fixing lychee link checks. The PR should either:

  • Split these changes into a separate PR with its own description, or
  • Update the PR description to explain why these workflow changes are included

This is important for maintaining clear change history and proper code review.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplifying allowedTools from specific command patterns (like "Bash(gh pr view:*)", "Bash(cat:*)", etc.) to just "Bash" significantly broadens the permissions granted to the Claude Code action.

This change allows the action to run ANY bash command, not just the previously whitelisted set. This is a security risk because:

  1. It removes the principle of least privilege
  2. Combined with --dangerouslyDisableSandbox, this creates a wider attack surface
  3. It's running in pull_request_target context with access to secrets

Consider:

  • Keeping the explicit whitelist of allowed commands
  • Documenting which specific additional commands are needed if the old whitelist was insufficient
  • Evaluating whether this change is truly necessary
Suggested change
"Bash"
"Bash(ls:*)" "Bash(cat:*)" "Bash(find:*)" "Bash(head:*)" "Bash(xargs:*)" "Bash(gh pr view:*)" "Bash(gh pr diff:*)"

Copilot uses AI. Check for mistakes.

prompt: |
You are running in pull_request_target.
Expand Down
7 changes: 5 additions & 2 deletions .lychee.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ accept = ["200", "429", "502", "504"]

verbose = "error"

# Exclude sitemap file from scanning (it contains pre-publish production URLs)
exclude_path = ["sitemap\\.xml"]
# Exclude files from scanning
exclude_path = [
"sitemap\\.xml", # Contains pre-publish production URLs
"404\\.html", # Root-relative links (/, /documentation/) can't resolve in local checks
]

# Exclude URLs that fail due to external issues (broken SSL, pre-deploy only, etc.)
exclude = [
Expand Down
Loading