Skip to content

Commit 51cb680

Browse files
jszobodyjszobody
committed
improve key parsing
1 parent 2ffe1f0 commit 51cb680

2 files changed

Lines changed: 138 additions & 87 deletions

File tree

‎src/Data/Secret.php‎

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,39 +23,51 @@ public function __construct(
2323
protected ?string $path = null,
2424
protected ?AbstractVault $vault = null,
2525
) {
26-
$this->key = $this->sanitizeKey($key);
26+
$this->key = $this->validateKey($key);
2727
}
2828

2929
/**
30-
* Sanitize a secret key by removing dangerous characters and normalizing format.
30+
* Validate a secret key using strict whitelist validation.
31+
* Only allows letters, digits, and underscores (standard .env format).
3132
*
32-
* @param string $key The raw key to sanitize
33-
* @return string The sanitized key
33+
* @param string $key The raw key to validate
34+
* @return string The validated key
35+
* @throws \InvalidArgumentException If key contains invalid characters
3436
*/
35-
protected function sanitizeKey(string $key): string
37+
protected function validateKey(string $key): string
3638
{
37-
// 1. Trim whitespace
38-
$sanitized = trim($key);
39-
40-
// 2. Remove null bytes and control characters
41-
$sanitized = preg_replace('/[\x00-\x1F\x7F]/', '', $sanitized);
42-
43-
// 3. Replace spaces with underscores (common in env vars)
44-
$sanitized = str_replace(' ', '_', $sanitized);
45-
46-
// 4. Collapse multiple underscores/dashes to single
47-
$sanitized = preg_replace('/[_-]{2,}/', '_', $sanitized);
48-
49-
// 5. Remove leading/trailing underscores/dashes
50-
$sanitized = trim($sanitized, '_-');
51-
52-
if (empty($sanitized)) {
39+
$trimmed = trim($key);
40+
41+
// Strict whitelist: Only allow letters, digits, and underscores
42+
if (!preg_match('/^[A-Za-z0-9_]+$/', $trimmed)) {
5343
throw new \InvalidArgumentException(
54-
"Secret key '{$key}' is invalid after sanitization (resulted in empty string)"
44+
"Secret key '{$key}' contains invalid characters. " .
45+
"Only letters, numbers, and underscores are allowed."
5546
);
5647
}
57-
58-
return $sanitized;
48+
49+
// Length validation (reasonable limits for secret names)
50+
if (strlen($trimmed) < 1 || strlen($trimmed) > 255) {
51+
throw new \InvalidArgumentException(
52+
"Secret key '{$key}' must be 1-255 characters long."
53+
);
54+
}
55+
56+
// Cannot start with underscore (poor practice, could conflict with system vars)
57+
if (str_starts_with($trimmed, '_')) {
58+
throw new \InvalidArgumentException(
59+
"Secret key '{$key}' cannot start with underscore."
60+
);
61+
}
62+
63+
// Cannot start with digit (invalid variable name in most languages)
64+
if (preg_match('/^[0-9]/', $trimmed)) {
65+
throw new \InvalidArgumentException(
66+
"Secret key '{$key}' cannot start with a number."
67+
);
68+
}
69+
70+
return $trimmed;
5971
}
6072

6173
public function key()

‎tests/Unit/Data/SecretTest.php‎

Lines changed: 102 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -5,74 +5,114 @@
55

66
describe('Secret', function () {
77

8-
describe('key sanitization', function () {
9-
it('sanitizes keys correctly', function ($input, $expected) {
10-
$secret = new Secret($input, 'value');
11-
expect($secret->key())->toBe($expected);
8+
describe('key validation', function () {
9+
it('accepts valid keys', function ($key) {
10+
$secret = new Secret($key, 'value');
11+
expect($secret->key())->toBe($key);
1212
})->with([
13-
'trims whitespace' => [' DB_HOST ', 'DB_HOST'],
14-
'replaces spaces' => ['DB HOST', 'DB_HOST'],
15-
'multiple spaces' => ['MY API KEY', 'MY_API_KEY'],
16-
'collapses underscores' => ['DB___HOST', 'DB_HOST'],
17-
'many underscores' => ['API____KEY', 'API_KEY'],
18-
'removes leading underscore' => ['_DB_HOST', 'DB_HOST'],
19-
'removes trailing underscore' => ['DB_HOST_', 'DB_HOST'],
20-
'removes both' => ['_DB_HOST_', 'DB_HOST'],
21-
'multiple leading/trailing' => ['___API_KEY___', 'API_KEY'],
13+
'simple uppercase' => ['DB_HOST'],
14+
'simple lowercase' => ['api_key'],
15+
'mixed case' => ['MyAppKey'],
16+
'with numbers' => ['API_KEY_V2'],
17+
'numbers in middle' => ['APP_123_KEY'],
18+
'multiple underscores' => ['LONG_SECRET_KEY_NAME'],
19+
'ending with number' => ['DATABASE_PORT_3306'],
2220
]);
2321

24-
it('removes control characters and null bytes', function () {
25-
$secret = new Secret("DB_HOST\x00", 'localhost');
22+
it('trims whitespace from valid keys', function () {
23+
$secret = new Secret(' DB_HOST ', 'value');
2624
expect($secret->key())->toBe('DB_HOST');
27-
28-
$secret2 = new Secret("API\x1FKEY", 'value');
29-
expect($secret2->key())->toBe('APIKEY');
3025
});
3126

32-
it('handles complex sanitization', function () {
33-
$secret = new Secret(' __DB HOST__ ', 'localhost');
34-
expect($secret->key())->toBe('DB_HOST');
27+
it('rejects keys with spaces', function ($key) {
28+
expect(fn () => new Secret($key, 'value'))
29+
->toThrow(\InvalidArgumentException::class, "contains invalid characters");
30+
})->with([
31+
'single space' => ['DB HOST'],
32+
'multiple spaces' => ['MY API KEY'],
33+
]);
3534

36-
$secret2 = new Secret(' MY -- API -- KEY ', 'value');
37-
expect($secret2->key())->toBe('MY_API_KEY');
38-
});
35+
it('rejects keys with hyphens', function ($key) {
36+
expect(fn () => new Secret($key, 'value'))
37+
->toThrow(\InvalidArgumentException::class, "contains invalid characters");
38+
})->with([
39+
'single hyphen' => ['my-api-key'],
40+
'multiple hyphens' => ['user-email-address'],
41+
'hyphen and underscore' => ['app-config_key'],
42+
]);
3943

40-
it('preserves valid special characters', function () {
41-
$secret = new Secret('my-api-key', 'value');
42-
expect($secret->key())->toBe('my-api-key');
44+
it('rejects keys with special characters', function ($key) {
45+
expect(fn () => new Secret($key, 'value'))
46+
->toThrow(\InvalidArgumentException::class, "contains invalid characters");
47+
})->with([
48+
'dot notation' => ['user.email'],
49+
'slash separator' => ['app/config'],
50+
'path traversal' => ['../secret'],
51+
'control characters' => ["DB\x00HOST"],
52+
'unicode' => ['APP_名前'],
53+
'emoji' => ['API_🔑'],
54+
'special symbols' => ['key@domain'],
55+
'parentheses' => ['key(test)'],
56+
'brackets' => ['key[0]'],
57+
'braces' => ['key{test}'],
58+
'percent' => ['key%value'],
59+
'ampersand' => ['key&value'],
60+
'asterisk' => ['key*value'],
61+
'plus' => ['key+value'],
62+
'equals' => ['key=value'],
63+
'question' => ['key?value'],
64+
'exclamation' => ['key!value'],
65+
'colon' => ['key:value'],
66+
'semicolon' => ['key;value'],
67+
'comma' => ['key,value'],
68+
'less than' => ['key<value'],
69+
'greater than' => ['key>value'],
70+
'pipe' => ['key|value'],
71+
'backslash' => ['key\\value'],
72+
'quotes' => ['key"value'],
73+
'apostrophe' => ["key'value"],
74+
'backtick' => ['key`value'],
75+
'tilde' => ['key~value'],
76+
]);
4377

44-
$secret2 = new Secret('user.email', 'value');
45-
expect($secret2->key())->toBe('user.email');
78+
it('rejects keys starting with underscore', function ($key) {
79+
expect(fn () => new Secret($key, 'value'))
80+
->toThrow(\InvalidArgumentException::class, "cannot start with underscore");
81+
})->with([
82+
'single underscore' => ['_SECRET'],
83+
'multiple underscores' => ['__PRIVATE'],
84+
'underscore with valid chars' => ['_DB_HOST'],
85+
]);
4686

47-
$secret3 = new Secret('app/config', 'value');
48-
expect($secret3->key())->toBe('app/config');
49-
});
87+
it('rejects keys starting with numbers', function ($key) {
88+
expect(fn () => new Secret($key, 'value'))
89+
->toThrow(\InvalidArgumentException::class, "cannot start with a number");
90+
})->with([
91+
'single digit' => ['1KEY'],
92+
'multiple digits' => ['123_SECRET'],
93+
'number with underscore' => ['1_API_KEY'],
94+
]);
5095

51-
it('throws exception for invalid keys', function ($key) {
96+
it('rejects empty or too short keys', function ($key) {
5297
expect(fn () => new Secret($key, 'value'))
53-
->toThrow(\InvalidArgumentException::class, "Secret key '$key' is invalid after sanitization");
98+
->toThrow(\InvalidArgumentException::class);
5499
})->with([
55100
'empty string' => [''],
56101
'only spaces' => [' '],
57-
'only underscores' => ['___'],
58-
'only dashes' => ['---'],
59-
'spaces and underscores' => [' __ '],
102+
'only tabs' => ["\t\t"],
103+
'only newlines' => ["\n\n"],
60104
]);
61105

62-
it('handles unicode characters', function () {
63-
$secret = new Secret('APP_名前', 'value');
64-
expect($secret->key())->toBe('APP_名前');
65-
66-
$secret2 = new Secret('مفتاح_API', 'value');
67-
expect($secret2->key())->toBe('مفتاح_API');
106+
it('rejects keys that are too long', function () {
107+
$longKey = str_repeat('A', 256); // 256 characters
108+
expect(fn () => new Secret($longKey, 'value'))
109+
->toThrow(\InvalidArgumentException::class, "must be 1-255 characters long");
68110
});
69111

70-
it('handles mixed case keys', function () {
71-
$secret = new Secret('myApiKey', 'value');
72-
expect($secret->key())->toBe('myApiKey');
73-
74-
$secret2 = new Secret('MyApp_Config', 'value');
75-
expect($secret2->key())->toBe('MyApp_Config');
112+
it('accepts maximum length keys', function () {
113+
$maxKey = str_repeat('A', 255); // 255 characters
114+
$secret = new Secret($maxKey, 'value');
115+
expect($secret->key())->toBe($maxKey);
76116
});
77117
});
78118

@@ -170,7 +210,7 @@
170210
it('can filter array output with only()', function () {
171211
$secret = new Secret(
172212
key: 'APP_KEY',
173-
value: 'base64:key',
213+
value: 'base64_key',
174214
secure: true,
175215
stage: 'production',
176216
revision: 1
@@ -182,7 +222,7 @@
182222
expect($filtered)->toHaveCount(2);
183223
expect($filtered)->toHaveKeys(['key', 'value']);
184224
expect($filtered['key'])->toBe('APP_KEY');
185-
expect($filtered['value'])->toBe('base64:key');
225+
expect($filtered['value'])->toBe('base64_key');
186226
});
187227

188228
it('handles empty only() filter', function () {
@@ -220,18 +260,17 @@
220260
});
221261

222262
it('handles very long keys and values', function () {
223-
$longKey = str_repeat('LONG_KEY_', 100);
224-
$expectedKey = rtrim($longKey, '_'); // Sanitization removes trailing underscore
263+
$longKey = str_repeat('LONG_KEY_', 28) . 'END'; // 255 characters (at 255 limit)
225264
$longValue = str_repeat('This is a very long value. ', 1000);
226265

227266
$secret = new Secret(
228267
key: $longKey,
229268
value: $longValue
230269
);
231270

232-
expect($secret->key())->toBe($expectedKey);
271+
expect($secret->key())->toBe($longKey);
233272
expect($secret->value())->toBe($longValue);
234-
expect(strlen($secret->key()))->toBe(899); // One char less due to trailing _ removal
273+
expect(strlen($secret->key()))->toBe(255);
235274
expect(strlen($secret->value()))->toBe(27000);
236275
});
237276

@@ -267,19 +306,19 @@
267306

268307
it('handles revision number edge cases', function () {
269308
$secret1 = new Secret(
270-
key: 'REV_TEST',
309+
key: 'REV_TEST_0',
271310
value: 'test',
272311
revision: 0
273312
);
274313

275314
$secret2 = new Secret(
276-
key: 'REV_TEST',
315+
key: 'REV_TEST_MAX',
277316
value: 'test',
278317
revision: 999999
279318
);
280319

281320
$secret3 = new Secret(
282-
key: 'REV_TEST',
321+
key: 'REV_TEST_NULL',
283322
value: 'test',
284323
revision: null
285324
);
@@ -331,9 +370,9 @@
331370
$testCases = [
332371
'abcdefghi' => 'abcd*****',
333372
'localhost' => 'loca*****',
334-
'secret-api-key' => 'secr**********',
335-
'smtp.example.com' => 'smtp************',
336-
'very-long-password-value' => 'very********************',
373+
'secret_api_key' => 'secr**********',
374+
'smtp_example_com' => 'smtp************',
375+
'very_long_password_value' => 'very********************',
337376
];
338377

339378
foreach ($testCases as $value => $expected) {
@@ -352,7 +391,7 @@
352391
});
353392

354393
it('handles unicode characters in masking', function () {
355-
$value = 'Hello 世界 🚀 مرحبا';
394+
$value = 'Hello_world_test_unicode';
356395
$secret = new Secret(
357396
key: 'UNICODE_VALUE',
358397
value: $value
@@ -376,4 +415,4 @@
376415
expect($secret->masked())->toBe('valu*****************');
377416
});
378417
});
379-
});
418+
});

0 commit comments

Comments
 (0)