Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/deploy-tooling-prevent-cleartext-credentials.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@sap-ux/deploy-tooling": patch
---

fix(deploy-tooling): prevent cleartext credentials in deploy configuration
33 changes: 29 additions & 4 deletions packages/deploy-tooling/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -129,8 +154,8 @@ Options:
--cloud target is an ABAP Cloud system
--cloud-service-key <file-location> 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-request> Transport number to record the change in the ABAP system
Expand Down Expand Up @@ -206,8 +231,8 @@ Options:
--cloud target is an ABAP Cloud system
--cloud-service-key <file-location> 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-request> Transport number to record the change in the ABAP system
--create-transport Create a transport request during deployment
Expand Down
17 changes: 17 additions & 0 deletions packages/deploy-tooling/src/base/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<AbapDeployConfig['credentials']>): 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.
*
Expand Down Expand Up @@ -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;
}
77 changes: 77 additions & 0 deletions packages/deploy-tooling/test/unit/base/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
});
9 changes: 9 additions & 0 deletions packages/deploy-tooling/test/unit/cli/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([
{
Expand Down Expand Up @@ -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) {
Expand Down
Loading