Skip to content

test(ssh): make injection tests self-contained (CTO-5387 follow-up) - #117

Merged
vedharish merged 1 commit into
mainfrom
fix/ssh-test-self-contained-config
Oct 28, 2026
Merged

vedharish merged 1 commit into
mainfrom
fix/ssh-test-self-contained-config

Conversation

@shrish-browserstack

@shrish-browserstack shrish-browserstack commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Why

ssh/test_ssh_injection.py::test_ssh_helper_rejects_malicious_key_without_connecting reads ACCESS_MODULES["ssh"]["app_user"] through get_username() (the label uses access_level: "app"). Enigma's CI builds its config from config.json.sample, which has no ssh module block, so the test dies with KeyError: 'ssh'.

Enigma's Unit Tests and Lint workflow clones this repo and runs its tests, so this one failure has turned enigma main red since the injection fix landed, and with it every open Dependabot PR on enigma.

RCA — how #113 shipped a test that fails in CI

What the test hits at runtime. The test exercises the app access level (the shared app-user path). In sshHelper() the username is resolved before the key is validated, and for app that means get_username() returns ACCESS_MODULES["ssh"]["app_user"] rather than the requester's own name. ACCESS_MODULES is the access_modules block of the host app's config.json. In enigma CI that file is a verbatim copy of config.json.sample, whose access_modules block holds only git_urls — there is no ssh section, so the lookup raises before the assertion under test is ever reached.

Why only this one test. The sibling malicious-username test uses access_level: "sudo", which falls back to user.user.username and never touches config. The validator and sink-quoting tests call the helpers directly. The app choice was incidental to the test's intent, which is why the fix here supplies the config to the test module rather than changing product code.

Why nothing caught it before merge.

  • This repo has no unit-test workflow — only semgrep.yml and CodeQL. The checks on #113 were semgrep, CodeQL and CodeRabbit; pytest never ran. These tests only execute when enigma's .github/workflows/unit-tests.yml clones this repo at run time.
  • fix(ssh): prevent OS command injection via ssh key and username (CTO-5387) #113 was verified locally against a developer config.json, which carries a real ssh block, so the suite was green on that machine.
  • Enigma's clone of this repo is unpinned (git_urls has no #<sha>), so the merge reached enigma CI on the next push with no change on the enigma side.

Timeline (2026-09-24, UTC).

Time Event
05:26 Last green enigma main run (35959955477) — cloned this repo before the test existed
05:47 #113 merged to main here
20:35 First red enigma run (36055856431); every enigma run since fails on this one test

Follow-ups (not in this PR).

  • Add a pytest workflow to this repo that mirrors enigma's clone-and-test sequence, so module tests run before merge.
  • Optionally have enigma pin the module clone to a commit SHA (<git-url>#<40-char-sha>, already supported by scripts/clone_access_modules.py), so a module merge cannot turn enigma red without an enigma-side change.

Change

An autouse fixture in the test module patches helpers.ACCESS_MODULES with a minimal ssh block (mocker.patch.dict, restored after each test). Test-only; no production code touched.

Verification

Ran the file against stubs that mirror enigma CI (ACCESS_MODULES = {"git_urls": []}), with real fabric:

main:        1 failed, 30 passed   (KeyError at ssh/helpers.py get_username)
this branch: 31 passed

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Central YAML (base), Organization UI (inherited), Workspace UI (inherited); Enforced overrides: Organization
  • Review profile: ASSERTIVE
  • Plan: Enterprise
  • Run ID: 709d314e-be3e-442e-b606-2a7ecf3ac3d7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@vedharish
vedharish merged commit dfb5a55 into main Oct 28, 2026
6 of 7 checks passed
@vedharish
vedharish deleted the fix/ssh-test-self-contained-config branch October 28, 2026 06:14
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.

2 participants