Skip to content

Commit 783aee4

Browse files
leoromanovskyleoromanovsky
committed
fix(openfeature): never send API keys to custom endpoints
1 parent 51cae1b commit 783aee4

7 files changed

Lines changed: 17 additions & 41 deletions

File tree

‎integration-tests/openfeature/openfeature-configuration-sources.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ function assertDeliveryTraffic (testCase) {
341341
for (const request of cdnRequests) {
342342
assert.strictEqual(request.url, `${AGENTLESS_PATH}?case=${testCase.identifier}`, testCase.label)
343343
assert.strictEqual(request.headers['accept-encoding'], 'gzip', testCase.label)
344-
assert.strictEqual(request.headers['dd-api-key'], 'integration-api-key', testCase.label)
344+
assert.strictEqual(request.headers['dd-api-key'], undefined, testCase.label)
345345
assert.strictEqual(request.headers['dd-client-library-language'], 'nodejs', testCase.label)
346346
assert.strictEqual(request.headers['dd-client-library-version'], VERSION, testCase.label)
347347
}

‎packages/dd-trace/src/exporters/common/request.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,11 @@ function request (data, options, callback) {
5050
}
5151

5252
// Never put the Datadog API key on a cleartext connection to a non-loopback host; that would
53-
// expose it on the wire. Loopback (local agent, dev proxy, tests) and explicit operator-owned
54-
// endpoints are exempt. Strip the key
53+
// expose it on the wire. Loopback (local agent, dev proxy, tests) is exempt. Strip the key
5554
// rather than drop the request: the agent proxies telemetry with its own key, while an https
5655
// intake URL is required to authenticate agentless traffic.
5756
const hasApiKey = options.headers['dd-api-key'] !== undefined || options.headers['DD-API-KEY'] !== undefined
58-
const insecureApiKey = options.protocol === 'http:' && !isLoopbackHost(options.hostname)
59-
if (hasApiKey && insecureApiKey && !options.allowInsecureApiKey) {
57+
if (hasApiKey && options.protocol === 'http:' && !isLoopbackHost(options.hostname)) {
6058
log.error(
6159
'Not sending the Datadog API key over a non-TLS connection to %s. Configure an https intake URL.',
6260
options.hostname

‎packages/dd-trace/src/openfeature/agentless_configuration_source.js‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ const RETRY_JITTER = 0.2
1818
/**
1919
* @typedef {object} AgentlessSourceConfig
2020
* @property {URL} endpoint
21-
* @property {boolean} allowInsecureApiKey
2221
* @property {number} pollIntervalMs
2322
* @property {number} requestTimeoutMs
2423
* @property {string | undefined} apiKey
@@ -165,8 +164,6 @@ class AgentlessConfigurationSource {
165164
url: this.#config.endpoint,
166165
method: 'GET',
167166
headers,
168-
// An explicit operator override may be a cleartext development or dogfooding proxy.
169-
allowInsecureApiKey: this.#config.allowInsecureApiKey,
170167
retry: false,
171168
signal,
172169
timeout: this.#config.requestTimeoutMs,
@@ -231,10 +228,7 @@ class AgentlessConfigurationSource {
231228
this.#failureWarnings.add(category)
232229

233230
if (statusCode === 401 || statusCode === 403) {
234-
log.warn(
235-
'Feature Flagging agentless endpoint returned HTTP %d; verify DD_API_KEY is configured and valid',
236-
statusCode
237-
)
231+
log.warn('Feature Flagging agentless endpoint returned HTTP %d; verify endpoint authentication', statusCode)
238232
} else if (statusCode) {
239233
log.warn('Feature Flagging agentless endpoint returned HTTP %d after %d attempts', statusCode, attempts)
240234
} else if (attempts > 1) {

‎packages/dd-trace/src/openfeature/configuration_source.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,9 @@ function create (config, applyConfiguration) {
3636
const AgentlessConfigurationSource = require('./agentless_configuration_source')
3737
return new AgentlessConfigurationSource({
3838
endpoint: endpoint(config, baseUrl),
39-
allowInsecureApiKey: hasCustomEndpoint,
4039
pollIntervalMs: Math.min(pollIntervalSeconds, MAX_POLL_INTERVAL_SECONDS) * 1000,
4140
requestTimeoutMs: requestTimeoutSeconds * 1000,
42-
apiKey: config.DD_API_KEY,
41+
apiKey: hasCustomEndpoint ? undefined : config.DD_API_KEY,
4342
}, applyConfiguration)
4443
} catch (error) {
4544
log.error('Unable to configure Feature Flagging configuration source', error)

‎packages/dd-trace/test/exporters/common/request.spec.js‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -847,23 +847,6 @@ describe('request', function () {
847847
})
848848
})
849849

850-
it('keeps dd-api-key over http for an explicitly trusted custom endpoint', (done) => {
851-
nock('http://flags.dev.internal', { reqheaders: { 'dd-api-key': 'secret-key' } })
852-
.post('/v1/input')
853-
.reply(200, 'OK')
854-
855-
request(Buffer.from(''), {
856-
method: 'POST',
857-
url: new URL('http://flags.dev.internal/v1/input'),
858-
headers: { 'dd-api-key': 'secret-key' },
859-
allowInsecureApiKey: true,
860-
}, (err, res) => {
861-
assert.strictEqual(res, 'OK')
862-
sinon.assert.notCalled(log.error)
863-
done(err)
864-
})
865-
})
866-
867850
for (const loopbackHost of ['127.0.0.1', '127.1.2.3', 'localhost', '[::1]']) {
868851
it(`keeps dd-api-key over http to the loopback host ${loopbackHost}`, (done) => {
869852
nock(`http://${loopbackHost}:9999`, {

‎packages/dd-trace/test/openfeature/agentless_configuration_source.spec.js‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ describe('AgentlessConfigurationSource', () => {
4848
applyConfiguration = sinon.stub()
4949
config = {
5050
endpoint: new URL('http://127.0.0.1:8080/api/v2/feature-flagging/config/rules-based/server'),
51-
allowInsecureApiKey: true,
5251
pollIntervalMs: 30_000,
5352
requestTimeoutMs: 2000,
5453
apiKey: 'test-api-key',
@@ -147,7 +146,6 @@ describe('AgentlessConfigurationSource', () => {
147146
assert.strictEqual(requests[0].data, '')
148147
assert.strictEqual(requests[0].options.url, config.endpoint)
149148
assert.strictEqual(requests[0].options.method, 'GET')
150-
assert.strictEqual(requests[0].options.allowInsecureApiKey, config.allowInsecureApiKey)
151149
assert.strictEqual(requests[0].options.retry, false)
152150
assert.strictEqual(requests[0].options.timeout, 2000)
153151
assert.strictEqual(requests[0].options.headers['DD-API-KEY'], 'test-api-key')
@@ -184,10 +182,11 @@ describe('AgentlessConfigurationSource', () => {
184182
clock = undefined
185183
const body = zlib.gzipSync(responseBody())
186184
config.endpoint = new URL('http://flags.dev.internal:8080/custom/ufc')
185+
delete config.apiKey
187186
nock('http://flags.dev.internal:8080', {
187+
badheaders: ['dd-api-key'],
188188
reqheaders: {
189189
'accept-encoding': 'gzip',
190-
'dd-api-key': 'test-api-key',
191190
},
192191
})
193192
.get('/custom/ufc')
@@ -417,7 +416,7 @@ describe('AgentlessConfigurationSource', () => {
417416
'third',
418417
])
419418
assert.deepStrictEqual(log.warn.thirdCall.args, [
420-
'Feature Flagging agentless endpoint returned HTTP %d; verify DD_API_KEY is configured and valid',
419+
'Feature Flagging agentless endpoint returned HTTP %d; verify endpoint authentication',
421420
401,
422421
])
423422
})
@@ -483,7 +482,7 @@ describe('AgentlessConfigurationSource', () => {
483482
assert.strictEqual(requests.length, 1)
484483
sinon.assert.calledOnceWithExactly(
485484
log.warn,
486-
'Feature Flagging agentless endpoint returned HTTP %d; verify DD_API_KEY is configured and valid',
485+
'Feature Flagging agentless endpoint returned HTTP %d; verify endpoint authentication',
487486
401
488487
)
489488

@@ -504,7 +503,7 @@ describe('AgentlessConfigurationSource', () => {
504503
assert.strictEqual(Object.hasOwn(requests[0].options.headers, 'DD-API-KEY'), false)
505504
sinon.assert.calledOnceWithExactly(
506505
log.warn,
507-
'Feature Flagging agentless endpoint returned HTTP %d; verify DD_API_KEY is configured and valid',
506+
'Feature Flagging agentless endpoint returned HTTP %d; verify endpoint authentication',
508507
401
509508
)
510509
sinon.assert.notCalled(applyConfiguration)

‎packages/dd-trace/test/openfeature/configuration_source.spec.js‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ describe('OpenFeature configuration source', () => {
5252
'https://ufc-server.ff-cdn.datadoghq.com/api/v2/feature-flagging/config/rules-based/server?dd_env=my+env'
5353
)
5454
assert.strictEqual(resolved.apiKey, 'test-api-key')
55-
assert.strictEqual(resolved.allowInsecureApiKey, false)
5655
assert.strictEqual(resolved.pollIntervalMs, 30_000)
5756
assert.strictEqual(resolved.requestTimeoutMs, 5000)
5857
})
@@ -87,10 +86,12 @@ describe('OpenFeature configuration source', () => {
8786
it(`allows the loopback endpoint ${baseUrl}`, () => {
8887
config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL = baseUrl
8988

89+
const resolved = createSourceConfig()
9090
assert.strictEqual(
91-
createSourceConfig().endpoint.toString(),
91+
resolved.endpoint.toString(),
9292
`${baseUrl}/api/v2/feature-flagging/config/rules-based/server`
9393
)
94+
assert.strictEqual(resolved.apiKey, undefined)
9495
})
9596
}
9697

@@ -103,17 +104,19 @@ describe('OpenFeature configuration source', () => {
103104
resolved.endpoint.toString(),
104105
'http://flags.dev.internal:8080/api/v2/feature-flagging/config/rules-based/server'
105106
)
106-
assert.strictEqual(resolved.allowInsecureApiKey, true)
107+
assert.strictEqual(resolved.apiKey, undefined)
107108
})
108109

109110
it('preserves an exact configured path and query', () => {
110111
config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL =
111112
'https://example.com/custom/ufc?tenant=one'
112113

114+
const resolved = createSourceConfig()
113115
assert.strictEqual(
114-
createSourceConfig().endpoint.toString(),
116+
resolved.endpoint.toString(),
115117
'https://example.com/custom/ufc?tenant=one'
116118
)
119+
assert.strictEqual(resolved.apiKey, undefined)
117120
})
118121

119122
it('derives and creates the managed GovCloud endpoint without hard-coding availability', () => {

0 commit comments

Comments
 (0)