Skip to content

fix(security): redact URL credentials, prevent report injection, harden HTTP handling - #4

Open
devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1787168044-security-hardening
Open

devin-ai-integration[bot] wants to merge 1 commit into
masterfrom
devin/1787168044-security-hardening

Conversation

@devin-ai-integration

Copy link
Copy Markdown

Summary

Security review of the whole codebase plus fixes for the issues that are actually exploitable. This is a CLI/GitHub Action with no server, database, or templating, so the SQLi / CORS / debug-endpoint / auth classes don't apply; the real attack surface is untrusted content coming out of scanned Markdown (which, in the Action, is attacker-controlled on a fork PR) and untrusted HTTP responses.

Fixed:

  1. Credential leak into logs, JSON reports, and PR comments. A link like https://user:token@internal.example.com/ was echoed verbatim into the console report, report.json, the Step Summary, and the PR comment. New redactUrlCredentials() in utils.ts rewrites userinfo to *** and is applied on every output path (truncateUrl, toJsonReport for url/finalUrl/redirectChain, toMarkdownReport).

  2. Markdown/HTML injection into the bot's PR comment. The URL was interpolated into a fixed single-backtick span, and error text / file paths were interpolated raw:

    - lines.push(`#### ${statusEmoji(result)} \`${result.url}\``);
    - if (result.error) lines.push(`- Error: ${result.error}`);
    + lines.push(`#### ${statusEmoji(result)} ${inlineCode(redactUrlCredentials(result.url))}`);
    + if (result.error) lines.push(`- Error: ${inlineCode(result.error)}`);

    A URL containing a backtick (e.g. https://example.com/`</details><img src=x>) closed the span and let a PR author write arbitrary Markdown/HTML — including tracking images and content that hides the rest of the report — into a comment posted by the repo's own token. inlineCode() grows the fence past the longest backtick run in the value and collapses newlines, so it cannot be escaped.

  3. Redirects were followed to arbitrary protocols. performRequest resolved Location with new URL(loc, current) and re-requested it without re-validating the scheme, so a server could bounce the checker to file:///other schemes. Now revalidated with isValidUrl and rejected.

  4. Unbounded response bodies. The HEAD→GET fallback buffered the entire body in memory (a malicious or misbehaving host could OOM the runner). maxContentLength/maxBodyLength are now capped at 5 MB.

  5. github-token was never masked — added core.setSecret(token) so a non-default token passed via the input can't be printed by later log output.

  6. Vulnerable runtime dependency: @actions/github@7 pinned undici@5.29.0 (high-severity: request smuggling, unbounded decompression, several CRLF-injection advisories). Bumped to ^9.1.1, which resolves undici@6.28.0; npm audit fix also cleaned up the remaining non-breaking transitive advisories. No runtime vulnerabilities remain.

Checked and found clean: no hardcoded secrets/keys anywhere; no eval/child_process/dynamic code loading; config input validated by the existing zod schema; the Markdown regexes and the ignore-pattern matcher were fuzzed with adversarial inputs (long unterminated links, quote/backtick runs, 60 KB tokens) with no ReDoS blowup.

Not fixed, deliberately — the remaining npm audit findings are all dev-only (vitest/vite/esbuild/postcss), reachable only via the Vitest UI/dev server, which this repo never runs. The fix requires vitest@4, a breaking major published 2026-08-18 (too new to vet); worth doing as a separate dependency PR.

Also worth considering (out of scope here): the checker will happily fetch http://localhost:* / link-local addresses found in docs — harmless on GitHub-hosted runners, but an SSRF primitive for anyone running this Action on a self-hosted runner. A --block-private-hosts-style option (default on in the Action) would close it.

Link to Devin session: https://app.devin.ai/sessions/8b2c45bbb02c4cd498f0882796aba23e
Requested by: @lahcenassmira

… bump @actions/github

- redact user:password userinfo from console, JSON, and Markdown reports
- render untrusted URLs, file paths, and error text as unescapable inline
  code so scanned docs cannot inject Markdown/HTML into PR comments
- reject redirects to non-http(s) protocols and cap response bodies at 5MB
- mask the action github-token via core.setSecret
- upgrade @actions/github to ^9.1.1 to drop vulnerable undici 5.x
@lahcenassmira lahcenassmira self-assigned this Aug 19, 2026
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant