Skip to content

fix: harden supply-chain security — obfuscation scanner + advisory policy#458

Open
SongotenU wants to merge 1 commit into
Egonex-AI:mainfrom
SongotenU:sec/supply-chain-hardening
Open

fix: harden supply-chain security — obfuscation scanner + advisory policy#458
SongotenU wants to merge 1 commit into
Egonex-AI:mainfrom
SongotenU:sec/supply-chain-hardening

Conversation

@SongotenU

@SongotenU SongotenU commented Jun 15, 2026

Copy link
Copy Markdown

Summary

Hardens supply-chain security following real malicious PRs observed in this repo:

Changes

1. SECURITY.md updates

  • Direct link to Security Advisories page
  • Email fallback (security@egonex.ai) if advisories unavailable
  • Supply-chain attacks explicitly in-scope

2. Obfuscation scanner script (.github/scripts/scan-obfuscation.sh)

Detects patterns commonly found in malicious PRs:

  • Packed/self-decoding JS blobs
  • Large base64/hex-encoded strings
  • String-shuffle decoders
  • global['!'] / global = global hijacks
  • Hidden eval/Function calls
  • require rebinding via global
  • Build-config file targeting (astro/vite/next configs)

3. Supply-chain CI workflow (.github/workflows/supply-chain-scan.yml)

  • Runs on every PR (opened, synchronize, reopened)
  • Generates full PR diff → runs obfuscation scanner
  • Non-blocking: auto-comments when suspicious patterns found, but does NOT block merge (false-positive safety)
  • Explicitly runs for fork PRs (highest risk vector)

Test plan

…policy

- Add CI workflow (supply-chain-scan.yml) scanning PR diffs for obfuscation/supply-chain attack patterns
- Add .github/scripts/scan-obfuscation.sh: detection of packed code, hidden eval, global hijack, base64/hex payloads, build-config targeting
- Update SECURITY.md: actionable advisory link, email fallback, supply-chain as in-scope
- Non-blocking: scan runs on all PRs, auto-comments when suspicious patterns found but does not block merge
- Mitigates real-world supply-chain attacks seen in this repo (PR Egonex-AI#198, Egonex-AI#206, Egonex-AI#261)
@Lum1104

Lum1104 commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

@codex review this

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eaa63fe3c6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

echo "Diff lines: $(wc -l < /tmp/pr-diff.txt)"

- name: Run obfuscation scanner
run: bash .github/scripts/scan-obfuscation.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Run the scanner from trusted code

Because the job checks out the PR head before this line, a malicious fork PR can change .github/scripts/scan-obfuscation.sh in the same PR to simply exit 0 or omit the dangerous patterns, so the supply-chain scan is bypassed exactly in the untrusted PRs it is meant to catch. Run the scanner from the base repository/workflow checkout or otherwise pin the script before checking out untrusted code.

Useful? React with 👍 / 👎.

echo "Diff lines: $(wc -l < /tmp/pr-diff.txt)"

- name: Run obfuscation scanner
run: bash .github/scripts/scan-obfuscation.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Make the manual-review path actually non-blocking

When scan-obfuscation.sh finds any warning it exits 1, and GitHub Actions keeps the job failed even though the following if: failure() comment step runs; without continue-on-error or capturing the exit code manually, PRs that only modify package.json or a build config will get a red check rather than the documented non-blocking warning, which can block merges if this workflow is required.

Useful? React with 👍 / 👎.

Comment on lines +47 to +50
await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid posting fork warnings with a read-only token

In fork PRs, which this workflow explicitly targets, the pull_request GITHUB_TOKEN is read-only, while creating an issue/PR comment requires issues: write or pull-requests: write; this github.rest.issues.createComment call will fail with a permission error instead of surfacing the scanner warning to maintainers unless the notification is moved to a privileged follow-up workflow or another safe reporting mechanism.

Useful? React with 👍 / 👎.

@tirth8205

Copy link
Copy Markdown
Contributor

Reviewed with a focus on whether the scanner is sound, its false-positive/false-negative profile, and whether it's truly advisory vs blocking. The direction is good and the advisory-only framing is the right call, but the primary signal misfires in two places worth fixing before merge.

Significant

  1. The warning comment will 403 on fork PRs (supply-chain-scan.yml). The workflow uses the pull_request trigger with no permissions: block. For fork PRs, GitHub scopes GITHUB_TOKEN to read-only, so the actions/github-script issues.createComment call in the "Flag for manual review" step will fail with 403 and post nothing. The job will still go red (the scanner exits 1), but the explanatory comment — described in the PR as the main signal — won't appear on exactly the fork PRs you call out as the highest-risk vector. To comment on fork PRs you need pull_request_target (or a workflow_run companion) with issues: write/pull-requests: write, being careful not to check out/execute untrusted code in the privileged context.

  2. package.json in CONFIG_FILES will fire on nearly every release PR (scan-obfuscation.sh). The file-targeting check increments WARNINGS (→ exit 1 → comment) for any PR touching package.json. I tested it against a plain version-bump diff and it flags. Given that releases here bump the version across several manifest files, most release PRs will trip the scanner — that's alert fatigue that trains people to ignore the check. Consider limiting to auto-exec configs (astro/vite/next/webpack) or only flagging package.json when scripts/dependencies actually change.

Minor

  1. ['"][A-Za-z0-9+/=]{100,}['"] (base64 heuristic) matches data: URIs and any long quoted token (verified by test). paths-ignore skips svg/png/md but not *.min.js, dist bundles, or lockfiles, so inlined/minified content will produce false positives. (sha512- integrity hashes happen to be safe because the hyphen breaks the run.)

  2. The job if: condition …fork || github.event_name != 'pull_request_target' is always true under the pull_request trigger — the second operand can never be false here. The condition and its inline comment are dead/misleading.

  3. The global-assignment loop uses grep -l against a single file, so $match is always /tmp/pr-diff.txt. The message "Suspicious global assignment in build/config file: /tmp/pr-diff.txt" never names the real file, and the diff is the whole PR (not config-scoped), so "in build/config file" is inaccurate.

Nits

  • edited in the trigger types re-runs the full scan on title/body edits, which don't change the diff. opened, synchronize, reopened is enough.
  • Both new files are missing a trailing newline.

Question

  • git diff ${base.sha}..${head.sha} after checkout of the fork head: is base.sha guaranteed to be in the fetched object graph for fork PRs even with fetch-depth: 0? If not, the diff step could fail with fatal: bad object instead of scanning. Worth confirming against a real fork PR.

Overall: The SECURITY.md changes are accurate and clear, and keeping the scan non-blocking (red check + soft comment, not a required gate) is the correct design for a regex heuristic. The main thing holding it back is that the comment won't post on fork PRs as written, and the package.json rule will generate routine noise. Fix #1 and #2 and this is a worthwhile guardrail.

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.

3 participants