From 2504ac3572c02ba2da87ddc018f31bf66bb849b5 Mon Sep 17 00:00:00 2001 From: Pravin Barton <9560941+isc-pbarton@users.noreply.github.com> Date: Mon, 11 May 2026 11:42:29 -0400 Subject: [PATCH 1/2] feat: validate that SSH key file path is not a directory --- CHANGELOG.md | 1 + cls/SourceControl/Git/Settings.cls | 13 +++++++++++++ csp/gitprojectsettings.csp | 9 +++++++-- test/UnitTest/SourceControl/Git/Settings.cls | 15 +++++++++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7f30105..0a9ed6d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Option to view an individual file's history in the source control menu (#960) - Change context menu now lists IPM packages from all Git-enabled namespaces, prefixed with the namespace name (#952) - Pull event handler option in settings page now displays user-friendly names for options (#908) +- Validation that SSH key file path is not a directory when configuring Embedded Git (#943) ### Fixed - Changes to % routines mapped to the current namespace may now be added to source control and committed (#944) diff --git a/cls/SourceControl/Git/Settings.cls b/cls/SourceControl/Git/Settings.cls index 7f871933..2ecef38b 100644 --- a/cls/SourceControl/Git/Settings.cls +++ b/cls/SourceControl/Git/Settings.cls @@ -427,6 +427,11 @@ ClassMethod HasNamespaceWebApp(Output webAppDirectory) As %Boolean [ Internal ] Method OnAfterConfigure() As %Boolean [ Internal ] { set defaultPromptFlag = $$$DisableBackupCharMask + $$$TrapCtrlCMask + $$$EnableQuitCharMask + $$$DisableHelpCharMask + $$$DisableHelpContextCharMask + $$$TrapErrorMask + set sc = ..ValidatePrivateKeyFilePath(..privateKeyFile) + if $$$ISERR(sc) { + write !,$System.Status.GetErrorText(sc),! + quit + } if (..privateKeyFile '= "") && '##class(%File).Exists(..privateKeyFile) { set value = 1 set response = ##class(%Library.Prompt).GetYesNo("Do you wish to create a new SSH key pair?",.value,,defaultPromptFlag) @@ -508,6 +513,14 @@ Method OnAfterConfigure() As %Boolean [ Internal ] } } +ClassMethod ValidatePrivateKeyFilePath(path As %String) As %Status +{ + if (path '= "") && ##class(%File).DirectoryExists(path) { + quit $$$ERROR($$$GeneralError, "SSH key file path is a directory. Please specify the full path to the key file.") + } + quit $$$OK +} + Method ConfigureBinPath(ByRef path As %String) As %Boolean [ Internal ] { if (path = "") { return 1 } diff --git a/csp/gitprojectsettings.csp b/csp/gitprojectsettings.csp index c70f2e1c..94941d3d 100644 --- a/csp/gitprojectsettings.csp +++ b/csp/gitprojectsettings.csp @@ -352,7 +352,12 @@ body {
Set fileExists = ##class(%File).Exists(settings.privateKeyFile) - if (settings.privateKeyFile = "") { + set validateSC = ##class(SourceControl.Git.Settings).ValidatePrivateKeyFilePath(settings.privateKeyFile) + if $$$ISERR(validateSC) { + set class = "form-control is-invalid" + set divClass = "invalid-feedback" + set feedbackText = $System.Status.GetErrorText(validateSC) + } elseif (settings.privateKeyFile = "") { set class = "form-control" set divClass = "neutral-feedback" set feedbackText = "You must configure an SSH private key to be able to work with remotes. This should be set up as a deploy key / equivalent, authenticating the server, not a specific user." @@ -379,7 +384,7 @@ body {
- if (settings.privateKeyFile '= "") && fileExists { + if (settings.privateKeyFile '= "") && fileExists && '$$$ISERR(validateSC) { set pubKeyName = settings.privateKeyFile_".pub" if ##class(%File).Exists(pubKeyName) { set pubStream = ##class(%Stream.FileCharacter).%OpenId(pubKeyName) diff --git a/test/UnitTest/SourceControl/Git/Settings.cls b/test/UnitTest/SourceControl/Git/Settings.cls index 038e2dd6..60470972 100644 --- a/test/UnitTest/SourceControl/Git/Settings.cls +++ b/test/UnitTest/SourceControl/Git/Settings.cls @@ -160,4 +160,19 @@ Method %OnClose() As %Status quit $$$OK } +Method TestValidatePrivateKeyFilePath() +{ + // empty path is OK + do $$$AssertStatusOK(##class(SourceControl.Git.Settings).ValidatePrivateKeyFilePath("")) + + // nonexistent file path is OK (will trigger key-generation prompt separately) + do $$$AssertStatusOK(##class(SourceControl.Git.Settings).ValidatePrivateKeyFilePath("/nonexistent/path/id_rsa")) + + // a directory path is an error + set mgrDir = ##class(%File).NormalizeDirectory($System.Util.ManagerDirectory()) + set sc = ##class(SourceControl.Git.Settings).ValidatePrivateKeyFilePath(mgrDir) + do $$$AssertStatusNotOK(sc) + do $$$AssertTrue($System.Status.GetErrorText(sc) [ "SSH key file path is a directory") +} + } From dcea843ce03f83640ddffc55c098d47a49471697 Mon Sep 17 00:00:00 2001 From: Pravin Barton <9560941+isc-pbarton@users.noreply.github.com> Date: Tue, 12 May 2026 12:01:07 -0400 Subject: [PATCH 2/2] refactor validate ssh key path with code style improvements --- cls/SourceControl/Git/Settings.cls | 2 +- csp/gitprojectsettings.csp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cls/SourceControl/Git/Settings.cls b/cls/SourceControl/Git/Settings.cls index 2ecef38b..f6a65b10 100644 --- a/cls/SourceControl/Git/Settings.cls +++ b/cls/SourceControl/Git/Settings.cls @@ -429,7 +429,7 @@ Method OnAfterConfigure() As %Boolean [ Internal ] set defaultPromptFlag = $$$DisableBackupCharMask + $$$TrapCtrlCMask + $$$EnableQuitCharMask + $$$DisableHelpCharMask + $$$DisableHelpContextCharMask + $$$TrapErrorMask set sc = ..ValidatePrivateKeyFilePath(..privateKeyFile) if $$$ISERR(sc) { - write !,$System.Status.GetErrorText(sc),! + write !, $System.Status.GetErrorText(sc), ! quit } if (..privateKeyFile '= "") && '##class(%File).Exists(..privateKeyFile) { diff --git a/csp/gitprojectsettings.csp b/csp/gitprojectsettings.csp index 94941d3d..1f51ab2a 100644 --- a/csp/gitprojectsettings.csp +++ b/csp/gitprojectsettings.csp @@ -384,7 +384,7 @@ body { - if (settings.privateKeyFile '= "") && fileExists && '$$$ISERR(validateSC) { + if (settings.privateKeyFile '= "") && fileExists && $$$ISOK(validateSC) { set pubKeyName = settings.privateKeyFile_".pub" if ##class(%File).Exists(pubKeyName) { set pubStream = ##class(%Stream.FileCharacter).%OpenId(pubKeyName)