Repository navigation
test(ssh): make injection tests self-contained (CTO-5387 follow-up) - #117
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
|
This was referenced Oct 23, 2026
shrish-browserstack
marked this pull request as draft
October 23, 2026 11:28
shrish-browserstack
marked this pull request as ready for review
October 28, 2026 05:44
vedharish
approved these changes
Oct 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
ssh/test_ssh_injection.py::test_ssh_helper_rejects_malicious_key_without_connectingreadsACCESS_MODULES["ssh"]["app_user"]throughget_username()(the label usesaccess_level: "app"). Enigma's CI builds its config fromconfig.json.sample, which has nosshmodule block, so the test dies withKeyError: 'ssh'.Enigma's Unit Tests and Lint workflow clones this repo and runs its tests, so this one failure has turned enigma
mainred 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
appaccess level (the shared app-user path). InsshHelper()the username is resolved before the key is validated, and forappthat meansget_username()returnsACCESS_MODULES["ssh"]["app_user"]rather than the requester's own name.ACCESS_MODULESis theaccess_modulesblock of the host app'sconfig.json. In enigma CI that file is a verbatim copy ofconfig.json.sample, whoseaccess_modulesblock holds onlygit_urls— there is nosshsection, 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 touser.user.usernameand never touches config. The validator and sink-quoting tests call the helpers directly. Theappchoice 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.
semgrep.ymland CodeQL. The checks on #113 were semgrep, CodeQL and CodeRabbit; pytest never ran. These tests only execute when enigma's.github/workflows/unit-tests.ymlclones this repo at run time.config.json, which carries a realsshblock, so the suite was green on that machine.git_urlshas no#<sha>), so the merge reached enigma CI on the next push with no change on the enigma side.Timeline (2026-09-24, UTC).
mainrun (35959955477) — cloned this repo before the test existedmainhereFollow-ups (not in this PR).
<git-url>#<40-char-sha>, already supported byscripts/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_MODULESwith a minimalsshblock (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 realfabric:🤖 Generated with Claude Code