Skip to content

Chore/vuln scanning#546

Open
muchasxmaracas wants to merge 16 commits intoTermix-SSH:mainfrom
muchasxmaracas:chore/vuln-scanning
Open

Chore/vuln scanning#546
muchasxmaracas wants to merge 16 commits intoTermix-SSH:mainfrom
muchasxmaracas:chore/vuln-scanning

Conversation

@muchasxmaracas
Copy link

@muchasxmaracas muchasxmaracas commented Feb 7, 2026

Overview

  • [ ✅ ] Added:

  • Trivy dependency vulnerability scan -> .github/workflows/sec-check.yml

  • [ ✅] Updated:

  • eslint.config.js to align local linting with linting in CI job

  • Updated all npm dependencies and pinned two transient dependencies with HIGH + MEDIUM vulnerabilities

  • [ ✅] Fixed:

  • PR Check job

Changes Made

  • Added a pipeline job to scan for vulnerabilities in npm dependencies -> it is now being executed on pull request and push to main
  • PR Check CI job was essentially useless (local linting was fine but CI job crashed): the logic and the inclusions/exclusions in the linter config had to be updated a bit
  • Linting exclusions are handled in config instead of comments in code
  • Linting locally is now aligned with linting in the pipeline
  • State in HostTerminalTab was moved to top level to comply with React Hook rules
  • General linting and formatting applied according to config

Related Issues

Screenshots / Demos

Checklist

  • [ ✅] Code follows project style guidelines
  • [ n/a] Supports mobile and desktop UI/app (if applicable)
  • [ ✅] I have read Contributing.md
  • [✅ ] This is not a translation request. See docs

@LukeGus
Copy link
Member

LukeGus commented Feb 8, 2026

I don't understand the need to add the Trivvy security action, especially since its a very unknown security workflow that I dont trust. Dependabot is already in use, as you im sure have seen, as you added the action. Also, why did you change the pr check action to use ubuntu-latest instead of the blacksmith runner? As far as I'm concerned, this can be closed as it being unneccessairy.

@muchasxmaracas
Copy link
Author

Given that the project doesn't use any kind of security checks I wanted to contribute and make this more transparent.

The dependencies in the codebase had multiple vulnerabilities which Trivy can help make visible and I have used this feedback to update the affected dependencies.
Trivy is on the CNCF Landscape and Aqua Security is a reputable vendor in the security space.

ubuntu-latest was chosen randomly. I have since made a commit to revert this to the runner used previously.

@LukeGus
Copy link
Member

LukeGus commented Feb 8, 2026

What do you mean this project does not use any security checks?

https://github.com/Termix-SSH/Termix/blob/main/.github/dependabot.yml

image

As I mentioned before, Dependabot is in use and is by far the more popular and well-trusted source.

@muchasxmaracas
Copy link
Author

Sorry, then I might have overlooked that.

It might be good to have a second opinion though, as I'm not sure which databases Dependabot uses to claim vulnerabilities.

Anyway, if you don't want to merge the sec-check, at least take a look at the pr-check fix.
The last few dozen PRs were all technically not successful because the linter config had some issues.

@LukeGus
Copy link
Member

LukeGus commented Feb 10, 2026

The changes to the PR linter look good to me. it may be best if the linter stays on node 20 since that's what the rest of Termix is compiled to, but it does not really matter. If you update or re-submit this PR without the security linter added, I will merge it.

@ZacharyZcR
Copy link
Member

If you're ready to accept this branch merge, I can audit and clean it up once. Let me know anytime if you need it. @LukeGus

@jnctech
Copy link

jnctech commented Mar 16, 2026

Overview

  • [ ✅ ] Added:

  • Trivy dependency vulnerability scan -> .github/workflows/sec-check.yml

  • [ ✅] Updated:

  • eslint.config.js to align local linting with linting in CI job

  • Updated all npm dependencies and pinned two transient dependencies with HIGH + MEDIUM vulnerabilities

  • [ ✅] Fixed:

  • PR Check job

Changes Made

  • Added a pipeline job to scan for vulnerabilities in npm dependencies -> it is now being executed on pull request and push to main
  • PR Check CI job was essentially useless (local linting was fine but CI job crashed): the logic and the inclusions/exclusions in the linter config had to be updated a bit
  • Linting exclusions are handled in config instead of comments in code
  • Linting locally is now aligned with linting in the pipeline
  • State in HostTerminalTab was moved to top level to comply with React Hook rules
  • General linting and formatting applied according to config

Related Issues

Screenshots / Demos

Checklist

  • [ ✅] Code follows project style guidelines
  • [ n/a] Supports mobile and desktop UI/app (if applicable)
  • [ ✅] I have read Contributing.md
  • [✅ ] This is not a translation request. See docs

Hi @LukeGus, @muchasxmaracas,

I'm a homelab user who recently deployed Termix in a test environment to evaluate it as an SSH management platform. I came across this PR and the discussion here, and I wanted to share my perspective as someone in the target audience for this project.

First — Termix is an impressive project with a lot of potential, and I appreciate the work that's gone into it.

However, after reviewing this PR conversation alongside the two published security advisories (CVE-2025-59951 and the stored XSS/LFI), I've decided to uninstall Termix from my environment. My concern isn't with any single issue — it's with the overall approach to security contributions.

A few observations:

  • Trivy is not an unknown tool. It's a CNCF project maintained by Aqua Security and is one of the most widely adopted vulnerability scanners in the industry. Dismissing it as untrusted is a red flag for users evaluating this project's security posture.
  • Dependabot is valuable, but it only covers known CVEs in declared dependencies. It does not perform the kind of vulnerability scanning that Trivy provides, and it would not have caught either of the two security advisories already published against this project.
  • The contributor here found actual HIGH and MEDIUM vulnerabilities in the dependency tree that weren't being addressed. That's exactly the kind of contribution a security-sensitive project should welcome.
  • The PR Check CI job had apparently been broken for dozens of PRs without being noticed, which suggests CI results weren't being actively monitored.

For a tool that stores SSH credentials and provides terminal access to infrastructure, defence in depth isn't optional — it's expected. I'd respectfully recommend:

  1. Accepting Trivy (or an equivalent scanner like Snyk or Grype) as part of the CI pipeline
  2. Considering adding SAST tooling (e.g. CodeQL, SonarCloud, or Semgrep) for application-level security analysis
  3. Establishing a SECURITY.md with a clear vulnerability disclosure and response process beyond just the GitHub advisory form

I hope this feedback is useful. I'd genuinely like to see Termix succeed — it fills a real gap in the self-hosted space. But I can't deploy a credentials management tool where security contributions are treated as unnecessary.

Thanks for your time.

@BRAVO68WEB
Copy link

I think @jnctech is right as it benefit in long term as it grows trust and community adoption.

@muchasxmaracas
Copy link
Author

@jnctech Thank you for your perspective and verbalizing what I couldn't.

I have only installed Termix locally but I could already tell that's it's a really cool project and I want it to succeed as well, hence my PR.

Given my years of professional experience in the DevOps area, creating opinionated CI/CD templates and more for hundreds of engineers in my company which relies on security checks due to regulations I felt like this project could benefit a lot from my contributions.

I don't have much experience contributing to open-source projects but after receiving such an adversarial reaction to a well-meant contribution, my desire to contribute more has honestly vanished.

If you or anybody else has recommendations to other projects which appreciate security and CI/CD PRs, I'll gladly look into it.

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.

5 participants