added fix to resolve all PR review comments

This commit is contained in:
Hitesh Borase
2026-02-13 15:46:14 +05:30
parent 8736f4f364
commit 50a16435e6
5 changed files with 57 additions and 141 deletions
+1 -1
View File
@@ -20,7 +20,7 @@ on:
workflow_dispatch:
push:
tags:
- 'v1.0.0' # Triggers on version tags like v1.0.0, v1.0.1, etc.
- 'v*.*.*' # Triggers on any version tag (v1.0.0, v1.0.1, v2.0.0, etc.) — no manual update per release
jobs:
test:
+23 -26
View File
@@ -18,7 +18,7 @@ All secret flow happens strictly between the plugin container and Keeper, while
- Keeper Secrets Manager access with Application configured
- Harness CI account with project setup
- KSM configuration (one-time access token `US:...` or Base64-encoded token or JSON confign)
- KSM configuration (one-time access token `US:...` or Base64-encoded token or JSON config)
## About
@@ -62,29 +62,26 @@ pipeline:
image: dhborse/keeper-harness-plugin
settings:
secrets: |
VeYTRo-PHElAwfQT6f0TIA/field/password > DB_PASSWORD
VeYTRo-PHElAwfQT6f0TIA/field/login > DB_USERNAME
VeYTRo-PHElAwfQT6f0TIA/file/credentials.txt > FILE_DATA
RECORD_UID/field/password > PASSWORD
RECORD_UID/field/login > USERNAME
envVariables:
KSM_CONFIG: <+secrets.getValue("Test_File_secret")>
KSM_CONFIG: <+secrets.getValue("keeper_base64_secret")>
- step:
type: Run
name: Use_Keeper_Secrets
identifier: Use_Keeper_Secrets
name: Use_Secrets
identifier: Use_Secrets
spec:
image: alpine:3.20
shell: Sh
command: |
if [ -f /harness/secrets/DB_USERNAME ] && [ -f /harness/secrets/DB_PASSWORD ]; then
DB_USERNAME=$(cat /harness/secrets/DB_USERNAME)
DB_PASSWORD=$(cat /harness/secrets/DB_PASSWORD)
echo "Username: $DB_USERNAME"
echo "Password: $DB_PASSWORD"
fi
if [ -f /harness/secrets/FILE_DATA ]; then
FILE_DATA=$(cat /harness/secrets/FILE_DATA)
echo "File Data: $FILE_DATA"
if [ -f /harness/secrets/USERNAME ] && [ -f /harness/secrets/PASSWORD ]; then
USERNAME=$(cat /harness/secrets/USERNAME)
PASSWORD=$(cat /harness/secrets/PASSWORD)
echo "Username: $USERNAME"
echo "Password retrieved successfully"
else
echo "Error: Secret files not found"
exit 1
fi
```
@@ -113,10 +110,10 @@ Keeper Notation queries mapping secrets to destinations:
**Example:**
```yaml
secrets: |
VeYTRo-PHElAwfQT6f0TIA/field/password > DB_PASSWORD
VeYTRo-PHElAwfQT6f0TIA/field/login > DB_USERNAME
VeYTRo-PHElAwfQT6f0TIA/file/credentials.txt > FILE_DATA
RECORD_UID/field/password > PASSWORD
RECORD_UID/field/login > USERNAME
```
Replace `RECORD_UID` with the actual Record UID from your Keeper Vault.
## Keeper Notation Format
@@ -138,10 +135,11 @@ The destination defines where the secret is stored:
|--------|-------------|----------------|
| `VARIABLE_NAME` | Default output (recommended) | `/harness/secrets/VARIABLE_NAME` |
**Examples:**
**Example:**
```yaml
# Default: saves to /harness/secrets/DB_PASSWORD
VeYTRo-PHElAwfQT6f0TIA/field/password > DB_PASSWORD
# Saves to /harness/secrets/PASSWORD and /harness/secrets/USERNAME
RECORD_UID/field/password > PASSWORD
RECORD_UID/field/login > USERNAME
```
## Accessing Secrets
@@ -156,9 +154,8 @@ Secrets are stored in `/harness/secrets/` directory. Read them in subsequent ste
image: alpine:3.20
shell: Sh
command: |
DB_USERNAME=$(cat /harness/secrets/DB_USERNAME)
DB_PASSWORD=$(cat /harness/secrets/DB_PASSWORD)
FILE_DATA=$(cat /harness/secrets/FILE_DATA)
USERNAME=$(cat /harness/secrets/USERNAME)
PASSWORD=$(cat /harness/secrets/PASSWORD)
# Use secrets in your build/deploy process
```
+4 -65
View File
@@ -1,69 +1,8 @@
#!/bin/bash
set -e
# 1. Create a temporary file to store the secrets
# Using mktemp ensures the file name is unique and not guessable
SECRETS_FILE=$(mktemp)
# Run Node.js script (writes directly to /harness/secrets/ files; no stdout — safe even if entrypoint is bypassed)
node /app/src/index.js
# 2. Run the Node.js plugin
# Redirect STDOUT (secrets) to our file, and let STDERR (logs) flow to the console
node /app/src/index.js > "$SECRETS_FILE"
# 3. Securely process the secrets
# SECURITY NOTE: All secrets are written to /harness/outputs/ and /harness/secrets/
mkdir -p /harness/outputs /harness/secrets
while IFS= read -r line; do
# Skip empty lines
if [ -z "$line" ]; then
continue
fi
# Determine type: ENV:, OUT:, or default
if [[ "$line" =~ ^ENV: ]]; then
type="env"
line="${line#ENV:}"
elif [[ "$line" =~ ^OUT: ]]; then
type="out"
line="${line#OUT:}"
else
type="out" # Default to output variable
fi
# Parse the line: split on first '=' to get name and value
name="${line%%=*}"
value="${line#*=}"
# Remove surrounding single quotes from value if present
if [[ "$value" =~ ^\'.*\'$ ]]; then
value="${value#\'}"
value="${value%\'}"
fi
# Skip if name is empty
if [ -z "$name" ]; then
continue
fi
# Export for the current shell session (plugin container only - not passed to next steps)
export "$name=$value"
# Write to Harness CI Plugin Output (for output variables)
printf "%s=%s\n" "$name" "$value" >> /harness/outputs/outputs.txt
# For environment variables, also write to env_vars.txt for Harness to pick up
# These are available as output variables and can be referenced in envVariables section
if [ "$type" = "env" ]; then
printf "%s=%s\n" "$name" "$value" >> /harness/outputs/env_vars.txt
fi
# Write to file for direct access (bypasses Harness truncation)
echo -n "$value" > "/harness/secrets/${name}"
chmod 600 "/harness/secrets/${name}" # Restrict permissions to owner only
done < "$SECRETS_FILE"
# 4. Secure Clean up
rm -f "$SECRETS_FILE"
# 5. Hand over control to the Docker command (if any)
# Hand over control to the Docker command (if any)
exec "$@"
+5 -22
View File
@@ -46,7 +46,7 @@ const processToken = (rawToken) => {
token = Buffer.from(token, 'base64').toString('utf-8').trim();
} catch (e) {
// Not base64, use as-is
console.log(e);
core.warning('Base64 decode failed, using raw value');
}
}
@@ -84,12 +84,6 @@ const processToken = (rawToken) => {
const parseSecretMappings = () => {
return core.getMultilineInput('secrets').map(line => {
const [notation, destRaw] = splitInput(line);
if (destRaw.startsWith('env:')) {
return { notation, destination: destRaw.slice(4), destinationType: 'environment' };
}
if (destRaw.startsWith('file:')) {
return { notation, destination: destRaw.slice(5), destinationType: 'file' };
}
return { notation, destination: destRaw, destinationType: 'output' };
});
};
@@ -144,21 +138,10 @@ const runPlugin = async () => {
Buffer.from(String(secret), 'utf8');
}
if (input.destinationType === 'file') {
const fullPath = path.resolve(input.destination);
fs.mkdirSync(path.dirname(fullPath), { recursive: true });
fs.writeFileSync(fullPath, data);
} else {
fs.mkdirSync('/harness/secrets', { recursive: true });
const secretFilePath = path.join('/harness/secrets', input.destination);
fs.writeFileSync(secretFilePath, data);
fs.chmodSync(secretFilePath, 0o600);
if (input.destinationType === 'environment') {
const outputValue = data.toString('utf8');
console.log(`ENV:${input.destination}='${outputValue}'`);
}
}
fs.mkdirSync('/harness/secrets', { recursive: true });
const secretFilePath = path.join('/harness/secrets', input.destination);
fs.writeFileSync(secretFilePath, data);
fs.chmodSync(secretFilePath, 0o600);
}
} catch (error) {
core.error(`Failed: ${error.message}`);
+22 -25
View File
@@ -261,9 +261,8 @@ describe('index.js - Complete Test Suite', () => {
expect(mockStderrWrite).toHaveBeenCalledWith(expect.stringContaining('INFO: Starting Keeper Secrets Manager plugin'));
});
test('should test console.log in base64 decode catch block', () => {
// This test ensures line 49 (console.log(e)) is covered
// Buffer.from doesn't throw for invalid base64, so we need to mock it to throw
test('should log base64 decode skip to stderr (no secret on stdout)', () => {
// Base64 decode failure is logged to stderr only, never stdout — avoids leaking to Docker logs
const originalBufferFrom = Buffer.from;
Buffer.from = jest.fn(() => {
throw new Error('Base64 decode error');
@@ -277,16 +276,14 @@ describe('index.js - Complete Test Suite', () => {
getValue.mockReturnValue('secret-value');
require('../index');
// The console.log in catch block should have been called
expect(mockConsoleLog).toHaveBeenCalled();
expect(mockStderrWrite).toHaveBeenCalledWith(expect.stringContaining('Base64 decode failed, using raw value'));
// Restore Buffer.from
Buffer.from = originalBufferFrom;
});
});
describe('parseSecretMappings', () => {
test('should parse env: destination type', async () => {
test('should treat env: prefix as literal destination name (no special parsing)', async () => {
process.env.KSM_CONFIG = 'US:test';
process.env.PLUGIN_SECRETS = 'notation>env:VAR_NAME';
localConfigStorage.mockReturnValue({});
@@ -296,7 +293,7 @@ describe('index.js - Complete Test Suite', () => {
require('../index');
await waitForAsync();
expect(mockConsoleLog).toHaveBeenCalledWith('ENV:VAR_NAME=\'secret-value\'');
expect(fs.writeFileSync).toHaveBeenCalledWith('/harness/secrets/env:VAR_NAME', expect.any(Buffer));
});
test('should parse file: destination type', async () => {
@@ -408,7 +405,7 @@ describe('index.js - Complete Test Suite', () => {
expect(fs.mkdirSync).toHaveBeenCalledWith('/app', { recursive: true });
});
test('should handle file destination type', async () => {
test('should treat file: prefix as literal destination (writes to /harness/secrets/)', async () => {
process.env.KSM_CONFIG = 'US:test';
process.env.PLUGIN_SECRETS = 'notation>file:/path/to/file.txt';
localConfigStorage.mockReturnValue({});
@@ -418,11 +415,10 @@ describe('index.js - Complete Test Suite', () => {
require('../index');
await waitForAsync();
expect(path.resolve).toHaveBeenCalledWith('/path/to/file.txt');
expect(fs.writeFileSync).toHaveBeenCalled();
expect(fs.writeFileSync).toHaveBeenCalledWith('/harness/secrets/file:/path/to/file.txt', expect.any(Buffer));
});
test('should create directory for file destination', async () => {
test('should create /harness/secrets for any destination', async () => {
process.env.KSM_CONFIG = 'US:test';
process.env.PLUGIN_SECRETS = 'notation>file:/path/to/file.txt';
localConfigStorage.mockReturnValue({});
@@ -433,7 +429,7 @@ describe('index.js - Complete Test Suite', () => {
require('../index');
await waitForAsync();
expect(fs.mkdirSync).toHaveBeenCalledWith('/path/to', { recursive: true });
expect(fs.mkdirSync).toHaveBeenCalledWith('/harness/secrets', { recursive: true });
});
test('should handle file secret with fileId', async () => {
@@ -695,7 +691,7 @@ describe('index.js - Complete Test Suite', () => {
});
describe('runPlugin - Destination Types', () => {
test('should handle environment destination type', async () => {
test('should handle env-prefixed destination as literal (writes to /harness/secrets/)', async () => {
process.env.KSM_CONFIG = 'US:test';
process.env.PLUGIN_SECRETS = 'notation>env:VAR_NAME';
localConfigStorage.mockReturnValue({});
@@ -705,13 +701,12 @@ describe('index.js - Complete Test Suite', () => {
require('../index');
await waitForAsync();
expect(mockConsoleLog).toHaveBeenCalledWith('ENV:VAR_NAME=\'env-value\'');
expect(fs.writeFileSync).toHaveBeenCalledWith('/harness/secrets/env:VAR_NAME', expect.any(Buffer));
expect(fs.mkdirSync).toHaveBeenCalledWith('/harness/secrets', { recursive: true });
expect(fs.writeFileSync).toHaveBeenCalled();
expect(fs.chmodSync).toHaveBeenCalled();
expect(fs.chmodSync).toHaveBeenCalledWith('/harness/secrets/env:VAR_NAME', 0o600);
});
test('should handle environment destination with Buffer data', async () => {
test('should handle env-prefixed destination with Buffer data', async () => {
process.env.KSM_CONFIG = 'US:test';
process.env.PLUGIN_SECRETS = 'notation>env:VAR_NAME';
localConfigStorage.mockReturnValue({});
@@ -721,22 +716,24 @@ describe('index.js - Complete Test Suite', () => {
require('../index');
await waitForAsync();
expect(mockConsoleLog).toHaveBeenCalledWith(expect.stringContaining('ENV:VAR_NAME'));
const call = fs.writeFileSync.mock.calls.find(c => c[0] === '/harness/secrets/env:VAR_NAME');
expect(call).toBeDefined();
expect(call[1].toString('utf8')).toBe('buffer-env-value');
});
test('should handle environment destination with non-Buffer data (line 157 branch)', async () => {
test('should handle env-prefixed destination with non-Buffer data', async () => {
process.env.KSM_CONFIG = 'US:test';
process.env.PLUGIN_SECRETS = 'notation>env:VAR_NAME';
localConfigStorage.mockReturnValue({});
initializeStorage.mockResolvedValue();
getSecrets.mockResolvedValue({});
// Return a number to test the String(data) branch
getValue.mockReturnValue(12345);
require('../index');
await waitForAsync();
expect(mockConsoleLog).toHaveBeenCalledWith(expect.stringContaining('ENV:VAR_NAME'));
expect(mockConsoleLog).toHaveBeenCalledWith(expect.stringContaining('12345'));
const call = fs.writeFileSync.mock.calls.find(c => c[0] === '/harness/secrets/env:VAR_NAME');
expect(call).toBeDefined();
expect(call[1].toString('utf8')).toBe('12345');
});
@@ -766,7 +763,7 @@ describe('index.js - Complete Test Suite', () => {
expect(fs.chmodSync).toHaveBeenCalledWith('/harness/secrets/output_var', 0o600);
});
test('should not output ENV: for non-environment destinations', async () => {
test('should write only to /harness/secrets for output destinations', async () => {
process.env.KSM_CONFIG = 'US:test';
process.env.PLUGIN_SECRETS = 'notation>output_var';
localConfigStorage.mockReturnValue({});
@@ -776,7 +773,7 @@ describe('index.js - Complete Test Suite', () => {
require('../index');
await waitForAsync();
expect(mockConsoleLog).not.toHaveBeenCalledWith(expect.stringContaining('ENV:'));
expect(fs.writeFileSync).toHaveBeenCalledWith('/harness/secrets/output_var', expect.any(Buffer));
});
});