mirror of
https://github.com/gradle/actions.git
synced 2026-09-10 19:28:33 +08:00
Relax Develocity access key format validation (#1061)
Develocity access keys have the format `server=key[;server=key]*`. The
short-lived-token code validated that format with:
```
/^([^;=\s]+=\w+)(;[^;=\s]+=\w+)*$/
```
This is too strict about `key`, and an OIDC token value fails it. There
were two independent problems:
1. `\w+` rejects the `.`, `-`, `_` and `=` padding a JWT contains.
2. Even had the regex passed, `parse` split each entry with
`hostKey.split('=')` and took `pair[1]`, so a key containing `=` would
have been **silently truncated** — a worse failure than rejection, since
the mangled key would then be sent to the server.
## Change
Drop `accessKeyRegexp` and the separate `isValid` gate; validate
structurally in `parse` instead, asserting only what is needed to split
the value:
- split on `;`, then split each entry on its **first** `=` only
- hostname: non-empty and free of whitespace (it cannot contain `=` or
`;` by construction)
- key: non-empty and free of whitespace — nothing else is assumed about
its shape
Two small intentional behaviour deltas beyond the fix:
- The whole value is now trimmed, so a trailing newline on a secret no
longer rejects the key. Internal whitespace still rejects.
- Empty entries (`host=key;`, `;host=key`) still reject, as before.
## Testing
Added cases to `short-lived-token.test.ts`: a JWT-shaped key value,
first-separator-only splitting (`host1=a=b==` → key `a=b==`),
surrounding-whitespace tolerance, a `raw()` round-trip preserving `==`
padding, and a table of nine rejection cases. 32 tests pass; prettier,
eslint and `./build` are clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
346a44564f
commit
02d4784b6e
@@ -147,7 +147,6 @@ type HostnameAccessKey = {
|
||||
}
|
||||
|
||||
export class DevelocityAccessCredentials {
|
||||
static readonly accessKeyRegexp = /^([^;=\s]+=\w+)(;[^;=\s]+=\w+)*$/
|
||||
readonly keys: HostnameAccessKey[]
|
||||
|
||||
private constructor(allKeys: HostnameAccessKey[]) {
|
||||
@@ -160,17 +159,63 @@ export class DevelocityAccessCredentials {
|
||||
|
||||
private static readonly keyDelimiter = ';'
|
||||
private static readonly hostDelimiter = '='
|
||||
private static readonly whitespace = /\s/
|
||||
|
||||
/**
|
||||
* Parse a `host=key[;host=key]*` access key value.
|
||||
*
|
||||
* Only the structure needed to split the value is validated: entries are separated by `;`, and
|
||||
* each entry is a hostname followed by `=` and a key, where the hostname contains no `=`, `;` or
|
||||
* whitespace, and the key is non-empty and contains no `;` or whitespace. Nothing else is
|
||||
* assumed about the key: it may be an OIDC token containing `.`, `-`, `_` and `=` padding, so
|
||||
* each entry is split on its _first_ `=` only.
|
||||
*
|
||||
* Returns `null` if the value doesn't match, emitting a warning that describes what is wrong.
|
||||
*/
|
||||
static parse(rawKey: string): DevelocityAccessCredentials | null {
|
||||
if (!this.isValid(rawKey)) {
|
||||
const trimmedKey = rawKey.trim()
|
||||
if (!trimmedKey) {
|
||||
return null
|
||||
}
|
||||
return new DevelocityAccessCredentials(
|
||||
rawKey.split(this.keyDelimiter).map(hostKey => {
|
||||
const pair = hostKey.split(this.hostDelimiter)
|
||||
return {hostname: pair[0], key: pair[1]}
|
||||
})
|
||||
const keys = new Array<HostnameAccessKey>()
|
||||
const entries = trimmedKey.split(this.keyDelimiter)
|
||||
for (const [index, entry] of entries.entries()) {
|
||||
const separatorIndex = entry.indexOf(this.hostDelimiter)
|
||||
if (separatorIndex === -1) {
|
||||
return this.warnBadlyFormed(index, entries.length, `no '${this.hostDelimiter}' separator`)
|
||||
}
|
||||
const hostname = entry.substring(0, separatorIndex)
|
||||
const key = entry.substring(separatorIndex + 1)
|
||||
if (!hostname) {
|
||||
return this.warnBadlyFormed(index, entries.length, 'empty server name')
|
||||
}
|
||||
if (!key) {
|
||||
return this.warnBadlyFormed(index, entries.length, 'empty key')
|
||||
}
|
||||
if (this.whitespace.test(hostname)) {
|
||||
return this.warnBadlyFormed(index, entries.length, 'whitespace in the server name')
|
||||
}
|
||||
if (this.whitespace.test(key)) {
|
||||
return this.warnBadlyFormed(index, entries.length, 'whitespace in the key')
|
||||
}
|
||||
keys.push({hostname, key})
|
||||
}
|
||||
return new DevelocityAccessCredentials(keys)
|
||||
}
|
||||
|
||||
/**
|
||||
* Warn that an access key value is badly formed and cannot be parsed. Reports only the position
|
||||
* of the offending entry and the reason: the value is a secret, and is not yet registered for
|
||||
* masking at this point, so no part of it is ever included in the message.
|
||||
*/
|
||||
private static warnBadlyFormed(index: number, entryCount: number, reason: string): null {
|
||||
const location = entryCount > 1 ? `entry ${index + 1} of ${entryCount}` : 'the value'
|
||||
core.warning(
|
||||
`Ignoring badly formed Develocity access key: ${reason} in ${location}. ` +
|
||||
`The expected format is 'server${this.hostDelimiter}key` +
|
||||
`[${this.keyDelimiter}server${this.hostDelimiter}key]*'.`
|
||||
)
|
||||
return null
|
||||
}
|
||||
|
||||
isEmpty(): boolean {
|
||||
@@ -182,10 +227,6 @@ export class DevelocityAccessCredentials {
|
||||
.map(k => `${k.hostname}${DevelocityAccessCredentials.hostDelimiter}${k.key}`)
|
||||
.join(DevelocityAccessCredentials.keyDelimiter)
|
||||
}
|
||||
|
||||
private static isValid(allKeys: string): boolean {
|
||||
return this.accessKeyRegexp.test(allKeys)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user