Skip to content

Fixes issue/js-lint (#2306)#3484

Open
kaitozaw wants to merge 5 commits intobeefproject:masterfrom
kaitozaw:issue/2306-js-lint
Open

Fixes issue/js-lint (#2306)#3484
kaitozaw wants to merge 5 commits intobeefproject:masterfrom
kaitozaw:issue/2306-js-lint

Conversation

@kaitozaw
Copy link
Collaborator

Pull Request

Category

Tests

Feature/Issue Description

Q: Please give a brief summary of your feature/fix
A:
Implements GitHub Actions workflow for JavaScript linting (Issue #2306) with hybrid approach: blocking job lints changed files only, non-blocking job reports on entire codebase. Prevents new technical debt while enabling gradual improvement of existing 6,500+ lint errors.

Q: Give a technical rundown of what you have changed (if applicable)
A:

  • Created .eslintrc.json with configuration tailored for BeEF's legacy codebase:

    • env: browser, es6: false - browser globals enabled, no ES6 for IE compatibility
    • ecmaVersion: 5 - parses ES5 syntax (pre-let/const/promises) to support older browsers
    • eslint:recommended - baseline rules without strict enforcement
    • ignorePatterns - excludes vendor libraries in core/main/client/lib/**
    • globals - defines BeEF framework globals (beef, jQuery/$/$j, MobileEsp, evercookie, etc.)
  • Created .github/workflows/eslint.yml with two parallel jobs:

    • lint-changed-files: Detects changed .js files via tj-actions/changed-files@v45, runs ESLint, blocks PR on errors
    • lint-all-files: Lints entire codebase with continue-on-error: true, report-only (non-blocking)
  • Updated package.json scripts: npm run lint and npm run lint:fix accept file arguments for flexible local testing

  • File pattern: **/*.js with exclusions for node_modules, core/main/client/lib, *.min.js

Test

Q: Describe your test cases, what you have covered and if there are any use cases that still need addressing.
A:

  • Local testing: Created test-lint.js with auto-fixable and non-fixable errors, verified npm run lint test-lint.js and npm run lint:fix test-lint.js work correctly
  • Workflow testing plan:
    1. Merge this PR to enable workflow
    2. Create test PR with clean JS changes → verify blocking job pass, while non-blocking fails (due to existing js fiiles)
    3. Create test PR with lint errors → verify blocking job fails, while non-blocking fails (due to existing js fiiles)
    4. Create test PR with no JS changes → verify lint steps skip gracefully

@kaitozaw kaitozaw temporarily deployed to Integrate Pull Request December 31, 2025 05:08 — with GitHub Actions Inactive
@kaitozaw kaitozaw linked an issue Jan 2, 2026 that may be closed by this pull request
@kaitozaw kaitozaw force-pushed the issue/2306-js-lint branch from 193586a to 3ba0ca3 Compare January 2, 2026 23:33
@kaitozaw kaitozaw temporarily deployed to Integrate Pull Request January 2, 2026 23:33 — with GitHub Actions Inactive
@kaitozaw kaitozaw temporarily deployed to Integrate Pull Request January 12, 2026 06:54 — with GitHub Actions Inactive
@zinduolis zinduolis temporarily deployed to Integrate Pull Request January 30, 2026 01:02 — with GitHub Actions Inactive
@zinduolis
Copy link
Contributor

Review for PR #3484

Overall, this is a great step forward in managing our JavaScript technical debt! The hybrid blocking/non-blocking approach is very pragmatic.

However, I've found a critical issue with the linter configuration regarding our module system, and a notification about our GitHub Actions versions.

🛑 Critical: ERB Templates in .js Files

I tested the linting workflow locally using act, and it fails on BeEF modules that use Ruby (ERB) templating within .js files.

The Issue:
Files like modules/social_engineering/sitekiosk_breakout/command.js contain Ruby tags (e.g., <%= @command_url %>). ESLint parses these files as strict JavaScript and throws fatal parsing errors (Unexpected token <), which will cause the Blocking job to fail if any of these files are touched in a PR.

Evidence (from local test run):

/home/runner/work/beef/beef/modules/social_engineering/sitekiosk_breakout/command.js
  18:46  error  Parsing error: Unexpected token <

/home/runner/work/beef/beef/modules/social_engineering/spoof_addressbar_data/command.js
  16:42  error  Parsing error: Unexpected token <

Recommended Fix:
We must exclude these files from the linter or use a plugin that can handle ERB. The simplest fix is to add modules/** to the ignorePatterns in .eslintrc.json.

"ignorePatterns": [
  "core/main/client/lib/**",
  "modules/**"
]

Alternatively, if we want to lint modules in the future, we would need a pre-processor, but ignoring them for now is safer to get this PR merged without breaking builds.

📦 GitHub Actions Versions

It is good practice to use the latest stable versions of actions to ensure we have the latest security patches and features.

Action Current Version Latest Version
actions/checkout v4 v6
actions/setup-node v4 v6
tj-actions/changed-files v45 v47

Please update .github/workflows/eslint.yml to use the latest versions:

- uses: actions/checkout@v6
- uses: actions/setup-node@v6
- uses: tj-actions/changed-files@v47

Summary

  • Fix: Add modules/** to .eslintrc.json ignore patterns to prevent ERB parsing errors.
  • Update: Bump actions/checkout to v6.
  • Update: Bump actions/setup-node to v6.
  • Update: Bump tj-actions/changed-files to v47.

@zinduolis zinduolis deployed to Integrate Pull Request February 5, 2026 00:40 — with GitHub Actions Active
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.

CI: Implement JS lint tests

2 participants