diff --git a/.changeset/deploy-tooling-prevent-cleartext-credentials.md b/.changeset/deploy-tooling-prevent-cleartext-credentials.md new file mode 100644 index 00000000000..3950d675ecf --- /dev/null +++ b/.changeset/deploy-tooling-prevent-cleartext-credentials.md @@ -0,0 +1,5 @@ +--- +"@sap-ux/deploy-tooling": patch +--- + +fix(deploy-tooling): prevent cleartext credentials in deploy configuration diff --git a/packages/deploy-tooling/README.md b/packages/deploy-tooling/README.md index 5245f0e03f0..fa3ac8b4ffc 100644 --- a/packages/deploy-tooling/README.md +++ b/packages/deploy-tooling/README.md @@ -72,6 +72,31 @@ This is the minimal custom task configuration for deployment using package `$TMP - /test/ ``` +#### Credentials + +You can provide credentials for the ABAP system using environment variables. The credentials are required for authentication against the ABAP system. Plain text credentials are not supported, use environment variables instead. + +```yaml +- name: abap-deploy-task + configuration: + credentials: + username: env:XYZ_USER + password: env:XYZ_PASSWORD + app: + name: Z_TEST + package: $TMP + target: + url: https://target.example + exclude: + - /test/ +``` +Create an `.env` file in the root of your project with the following content: + +``` +XYZ_USER=your-username +XYZ_PASSWORD=your-password +``` + #### Configuration with logging enabled Set the level of detail for log messages, default is `Info`; ```json @@ -129,8 +154,8 @@ Options: --cloud target is an ABAP Cloud system --cloud-service-key JSON file location with the ABAP cloud service key. --cloud-service-env Load ABAP cloud service properties from either a .env file or your environment variables. Secrets in your .env should not be committed to source control. - --username ABAP Service username - --password ABAP Service password + --username ABAP Service username, plain text credentials are not supported, use environment variables + --password ABAP Service password, plain text credentials are not supported, use environment variables --authentication-type Authentication type for the app (e.g. 'basic', 'reentranceTicket'). Required for 'reentranceTicket'. --create-transport Create a transport request during deployment/undeployment --transport Transport number to record the change in the ABAP system @@ -206,8 +231,8 @@ Options: --cloud target is an ABAP Cloud system --cloud-service-key JSON file location with the ABAP cloud service key. --cloud-service-env Load ABAP cloud service properties from either a .env file or your environment variables - --username ABAP Service username - --password ABAP Service password + --username ABAP Service username, plain text credentials are not supported, use environment variables + --password ABAP Service password, plain text credentials are not supported, use environment variables --authentication-type Authentication type for the app (e.g. 'basic', 'reentranceTicket'). Required for 'reentranceTicket'. --transport Transport number to record the change in the ABAP system --create-transport Create a transport request during deployment diff --git a/packages/deploy-tooling/src/base/config.ts b/packages/deploy-tooling/src/base/config.ts index bde59bd20a0..a0e45d79d3b 100644 --- a/packages/deploy-tooling/src/base/config.ts +++ b/packages/deploy-tooling/src/base/config.ts @@ -49,6 +49,20 @@ function validateTarget(target: AbapTarget): AbapTarget { return target; } +/** + * Validates that credentials are provided as environment variable references, not as plain text. + * + * @param credentials - credentials to validate + */ +function validateCredentials(credentials: NonNullable): void { + const isEnvRef = (value: string | undefined): boolean => !value || value.startsWith('env:'); + if (!isEnvRef(credentials.username) || !isEnvRef(credentials.password)) { + throw new Error( + 'Credentials must be provided as environment variable references, such as env:MY_VAR, not as plain text.' + ); + } +} + /** * Type checking the config object. * @@ -87,6 +101,9 @@ export function validateConfig(config: AbapDeployConfig | undefined, logger?: Lo if (!config.app) { throwConfigMissingError('app'); } + if (config.credentials) { + validateCredentials(config.credentials); + } return config; } diff --git a/packages/deploy-tooling/test/unit/base/config.test.ts b/packages/deploy-tooling/test/unit/base/config.test.ts index a5bb67fe1f0..951413c530d 100644 --- a/packages/deploy-tooling/test/unit/base/config.test.ts +++ b/packages/deploy-tooling/test/unit/base/config.test.ts @@ -131,5 +131,82 @@ describe('base/config', () => { expect(() => validateConfig(config)).not.toThrow(); expect(config.app.package).toBe('$TMP'); }); + + describe('validateCredentials', () => { + test('throws when username is plaintext (no env: prefix)', () => { + // given a config with a plaintext username + const config = { + ...validConfig, + credentials: { username: 'admin', password: 'env:MY_PASSWORD' } + } as AbapDeployConfig; + + // when validateConfig is called + // then it throws with the expected message + expect(() => validateConfig(config)).toThrow( + 'Credentials must be provided as environment variable references' + ); + }); + + test('throws when password is plaintext (no env: prefix)', () => { + // given a config with a plaintext password + const config = { + ...validConfig, + credentials: { username: 'env:MY_USER', password: 'secret' } + } as AbapDeployConfig; + + // when validateConfig is called + // then it throws with the expected message + expect(() => validateConfig(config)).toThrow( + 'Credentials must be provided as environment variable references' + ); + }); + + test('throws when both username and password are plaintext', () => { + // given a config with plaintext username and password + const config = { + ...validConfig, + credentials: { username: 'admin', password: 'secret' } + } as AbapDeployConfig; + + // when validateConfig is called + // then it throws with the expected message + expect(() => validateConfig(config)).toThrow( + 'Credentials must be provided as environment variable references' + ); + }); + + test('passes when both username and password use env: prefix', () => { + // given a config with env-referenced credentials + const config = { + ...validConfig, + credentials: { username: 'env:MY_USER', password: 'env:MY_PASSWORD' } + } as AbapDeployConfig; + + // when validateConfig is called + // then it does not throw + expect(() => validateConfig(config)).not.toThrow(); + }); + + test('passes when credentials are absent', () => { + // given a config without credentials + const config = { ...validConfig } as AbapDeployConfig; + + // when validateConfig is called + // then it does not throw + expect(() => validateConfig(config)).not.toThrow(); + }); + + test('passes when only username is set with env: prefix and password is absent', () => { + // given a config with only username set via env: + const config = { + ...validConfig, + credentials: { username: 'env:MY_USER' } + } as unknown as AbapDeployConfig; + + // when validateConfig is called + // then it does not throw + expect(() => validateConfig(config)).not.toThrow(); + }); + }); }); }); diff --git a/packages/deploy-tooling/test/unit/cli/index.test.ts b/packages/deploy-tooling/test/unit/cli/index.test.ts index c93a669b35f..2c8e5720b40 100644 --- a/packages/deploy-tooling/test/unit/cli/index.test.ts +++ b/packages/deploy-tooling/test/unit/cli/index.test.ts @@ -81,6 +81,7 @@ describe('cli', () => { const cliCmdWithUaa = [...cliCmd, '--cloud-service-env', '--service', '/bc/my/uaa/deploy/service']; const cliCmdWithAuthType = [...cliCmd, '--authentication-type', 'reentranceTicket']; const cliCmdWithConnectPath = [...cliCmd, '--connect-path', '/my/connect/path']; + const cliCmdWithBasicAuth = [...cliCmd, '--username', 'env:username', '--password', 'env:password']; test.each([ { @@ -127,11 +128,19 @@ describe('cli', () => { writeFileSyncCalled: 0, deployFn: mockedUi5RepoService.deploy, object: { connectPath: '/my/connect/path' } + }, + { + params: cliCmdWithBasicAuth, + writeFileSyncCalled: 0, + deployFn: mockedUi5RepoService.deploy, + object: { retry: false, strictSsl: false } } ])( 'successful deploy with different options %s', async ({ params, writeFileSyncCalled, object, provider, deployFn }) => { process.argv = params; + delete process.env.username; + delete process.env.password; await runDeploy(); expect(deployFn).toHaveBeenCalled(); if (provider) {