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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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

@muchasxmaracas
Copy link
Copy Markdown
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.

@jnctech
Copy link
Copy Markdown

jnctech commented Mar 22, 2026

Following up on my earlier comment about defence in depth — I want to address this with more nuance, because the security landscape has shifted since this PR was opened.

The Trivy supply chain compromise (March 19-20, 2026) is directly relevant here. An attacker compromised Trivy's release pipeline and distributed credential-stealing malware through trivy-action versions 0.0.1–0.34.2, the trivy container image (tag 0.69.4), and setup-trivy (full advisory). The malware targeted SSH keys, cloud credentials, API tokens, and registry configs — exactly the kind of secrets a tool like Termix handles.

This doesn't invalidate the case for vulnerability scanning — it strengthens it. The incident is a textbook example of why defence in depth matters. Any single tool can be compromised. What matters is layering controls so that a breach in one layer doesn't cascade. Dependabot alone is one layer. A vulnerability scanner is another. But both need to be consumed safely.

I've been hardening our own CI/CD pipelines in response to this incident, and I'd offer these concrete recommendations for Termix — whether Trivy or another scanner is adopted:

1. SHA-pin all action references (CRITICAL)

Every workflow in this repo uses mutable tags (@v4, @v5, @V3). This is the exact attack vector used in the Trivy compromise — an attacker overwrites a tag to point at malicious code, and every downstream consumer executes it on their next CI run.

Current (vulnerable to tag rewriting):

- uses: actions/checkout@v5

Hardened (immutable, auditable):

- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.2.2

This applies to actions/checkout, actions/setup-node, docker/login-action, docker/build-push-action, docker/setup-qemu-action, docker/setup-buildx-action, ruby/setup-ruby, actions/upload-artifact, and actions/download-artifact — all currently tag-referenced across docker.yml, electron.yml, pr-check.yml, and openapi.yml. Dependabot's github-actions ecosystem config (which you already have) will propose SHA-bump PRs automatically once you switch.

2. Restrict workflow permissions (HIGH)

None of the four workflows declare a permissions: block. This means every job runs with the default token permissions — broader than any single job needs. Best practice:

permissions: {}

jobs:
  build:
    permissions:
      contents: read

The docker.yml workflow is especially sensitive — it handles GHCR_TOKEN and DOCKERHUB_TOKEN and pushes container images. A compromised action in that workflow could push a trojaned Termix image to Docker Hub.

3. Disable credential persistence on checkout (HIGH)

actions/checkout persists the GITHUB_TOKEN in local git config by default. Any subsequent step — including a compromised action — can extract it:

- uses: actions/checkout@<sha>
  with:
    persist-credentials: false

4. On the scanner itself

I still support including vulnerability scanning in the pipeline — that position hasn't changed. Whether it's Trivy (pinned to v0.69.3 or the safe trivy-action@0.35.0), npm audit, or another tool, the principle is the same: Dependabot catches known CVEs in declared dependencies, but doesn't scan for misconfigurations, container image vulnerabilities, or transitive issues that a dedicated scanner catches.

For a project that stores SSH credentials, the attack surface justifies more than one layer of automated scanning. The Trivy incident doesn't argue against scanning — it argues for consuming all CI dependencies (scanners included) with proper integrity controls.

5. Quick wins for this repo

  • Add persist-credentials: false to all checkout steps
  • Add permissions: {} at the workflow level across all four workflow files
  • SHA-pin all uses: references (Dependabot will maintain them going forward)
  • Consider adding secret scanning (e.g., Gitleaks) given the SSH credential handling
  • Consider SBOM generation on releases for downstream transparency

I've built a reusable CI/CD supply chain review template based on the Trivy incident that covers all of the above with severity-rated checklists, remediation examples, and an incident response playbook — happy to share it.

@LukeGus
Copy link
Copy Markdown
Member

LukeGus commented Mar 29, 2026

Sorry, been extremely busy the past few weeks, but now I'm able to discuss this issue. However, I'm still not sold on Trivvy, especially considering the previously mentioned supply chain attack. However, that does not mean I'm not open to adding a form of repository-wide scanning or PR-checks. I just need to do some research into it to figure out the right path to go down.

@SandiyosDev
Copy link
Copy Markdown

oh, this sure has aged well

@jnctech
Copy link
Copy Markdown

jnctech commented Apr 3, 2026

What I'd like to see

I genuinely want Termix to succeed — it fills a real gap. But I think the project needs:

  • A vulnerability scanner in CI (Trivy, Snyk, or Grype)
  • SHA-pinned GitHub Actions and scoped workflow permissions
  • SAST tooling (CodeQL, Semgrep) for application-level analysis
  • A real SECURITY.md with a disclosure and response process (the current one is two lines)
  • At least one additional maintainer reviewing security-sensitive code
  • A published security model — how are credentials encrypted at rest?

@LukeGus
Copy link
Copy Markdown
Member

LukeGus commented Apr 4, 2026

A few of these have already been answered.

SAST: I use GitHub's new Code Quality feature, which auto-scans using AI and CodeQL
Maintainer: @ZacharyZcR Often co-reviews
Security Model: https://docs.termix.site/security

@ZacharyZcR
Copy link
Copy Markdown
Member

I'll get to this PR later and make sure it better meets our current needs.

@ZacharyZcR
Copy link
Copy Markdown
Member

In fact, we were almost ready to approve this PR, but the Trivy poisoning incident had such a significant impact that we need to spend more time evaluating 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.

6 participants