Skip to content

Commit 05df5bd

Browse files
committed
fix(openfeature): harden agentless delivery boundaries
Partial HTTP responses could leave polling pending because the shared transport settled only on end. Every terminal response event now settles once, and mixed-case gzip encodings remain supported. Cleartext non-loopback overrides could apply unauthenticated flag data after the transport stripped the API key. They are now rejected before a request, and diagnostics exclude sensitive endpoint and payload data. Feature Flags now activate independently from APM tracing, so Remote Config cannot target the noop provider. Inactive processes still avoid loading the provider, source, and agentless header helper.
1 parent 566993e commit 05df5bd

14 files changed

Lines changed: 341 additions & 118 deletions

File tree

index.d.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,12 @@ interface Tracer extends opentracing.Tracer {
156156
llmobs: tracer.llmobs.LLMObs;
157157

158158
/**
159-
* OpenFeature Provider with Remote Config integration.
159+
* OpenFeature Provider with agentless and Agent Remote Config delivery.
160160
*
161-
* Extends DatadogNodeServerProvider with Remote Config integration for dynamic flag configuration.
162-
* Enable with DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED=true.
161+
* Agentless delivery is enabled by default and starts when the provider is first accessed.
163162
*
164-
* @env DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED
163+
* @env DD_FEATURE_FLAGS_ENABLED
164+
* @env DD_FEATURE_FLAGS_CONFIGURATION_SOURCE
165165
* @beta This feature is in preview and not ready for production use
166166
*/
167167
openfeature: tracer.OpenFeatureProvider;
@@ -799,9 +799,9 @@ declare namespace tracer {
799799
*/
800800
flaggingProvider?: {
801801
/**
802-
* Whether to enable the feature flagging provider.
803-
* Requires Remote Config to be properly configured.
804-
* Can be configured via DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED environment variable.
802+
* Legacy feature flagging provider switch.
803+
* When the stable Feature Flags configuration is unset, true selects Agent Remote Config and false disables
804+
* the provider.
805805
*
806806
* @default false
807807
* @env DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED
@@ -1620,7 +1620,7 @@ declare namespace tracer {
16201620
/**
16211621
* Flagging Provider (OpenFeature-compatible).
16221622
*
1623-
* Wraps @datadog/openfeature-node-server with Remote Config integration for dynamic flag configuration.
1623+
* Wraps @datadog/openfeature-node-server with agentless and Agent Remote Config delivery.
16241624
* Implements the OpenFeature Provider interface for flag evaluation.
16251625
*
16261626
* @beta This feature is in preview and not ready for production use

index.d.v5.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,12 @@ interface Tracer extends opentracing.Tracer {
156156
llmobs: tracer.llmobs.LLMObs;
157157

158158
/**
159-
* OpenFeature Provider with Remote Config integration.
159+
* OpenFeature Provider with agentless and Agent Remote Config delivery.
160160
*
161-
* Extends DatadogNodeServerProvider with Remote Config integration for dynamic flag configuration.
162-
* Enable with DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED=true.
161+
* Agentless delivery is enabled by default and starts when the provider is first accessed.
163162
*
164-
* @env DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED
163+
* @env DD_FEATURE_FLAGS_ENABLED
164+
* @env DD_FEATURE_FLAGS_CONFIGURATION_SOURCE
165165
* @beta This feature is in preview and not ready for production use
166166
*/
167167
openfeature: tracer.OpenFeatureProvider;
@@ -869,9 +869,9 @@ declare namespace tracer {
869869
*/
870870
flaggingProvider?: {
871871
/**
872-
* Whether to enable the feature flagging provider.
873-
* Requires Remote Config to be properly configured.
874-
* Can be configured via DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED environment variable.
872+
* Legacy feature flagging provider switch.
873+
* When the stable Feature Flags configuration is unset, true selects Agent Remote Config and false disables
874+
* the provider.
875875
*
876876
* @default false
877877
* @env DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED
@@ -1732,7 +1732,7 @@ declare namespace tracer {
17321732
/**
17331733
* Flagging Provider (OpenFeature-compatible).
17341734
*
1735-
* Wraps @datadog/openfeature-node-server with Remote Config integration for dynamic flag configuration.
1735+
* Wraps @datadog/openfeature-node-server with agentless and Agent Remote Config delivery.
17361736
* Implements the OpenFeature Provider interface for flag evaluation.
17371737
*
17381738
* @beta This feature is in preview and not ready for production use

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

Lines changed: 49 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,11 @@
66
const { Readable } = require('stream')
77
const http = require('http')
88
const https = require('https')
9-
const net = require('node:net')
109
const zlib = require('zlib')
1110

1211
const { storage } = require('../../../../datadog-core')
1312
const log = require('../../log')
14-
const { parseUrl } = require('./url')
13+
const { isLoopbackHost, parseUrl } = require('./url')
1514
const docker = require('./docker')
1615
const { httpAgent, httpsAgent } = require('./agents')
1716
const {
@@ -27,18 +26,6 @@ const maxActiveBufferSize = 1024 * 1024 * 64
2726

2827
let activeBufferSize = 0
2928

30-
/**
31-
* @param {string} hostname Host as resolved by {@link parseUrl}; IPv6 is unbracketed (`::1`).
32-
* @returns {boolean}
33-
*/
34-
function isLoopbackHost (hostname) {
35-
// The 127.0.0.0/8 block is loopback, but only when the host is an actual IPv4 literal: a
36-
// hostname like `127.evil.com` shares the prefix yet resolves anywhere, so net.isIPv4 gates it.
37-
return hostname === 'localhost' ||
38-
hostname === '::1' ||
39-
(hostname.startsWith('127.') && net.isIPv4(hostname))
40-
}
41-
4229
/**
4330
* @param {Buffer|string|Readable|Array<Buffer|string>} data
4431
* @param {object} options
@@ -110,36 +97,48 @@ function request (data, options, callback) {
11097

11198
/**
11299
* @param {import('node:http').IncomingMessage} res
113-
* @param {() => void} finalize
100+
* @param {(error: Error|null, result?: string|null, statusCode?: number,
101+
* headers?: import('node:http').IncomingHttpHeaders) => void} complete
102+
* @param {(error: Error) => void} handleError
114103
*/
115-
const onResponse = (res, finalize) => {
104+
const onResponse = (res, complete, handleError) => {
116105
markEndpointReached(options)
117106

118107
const chunks = []
119108

120109
res.setTimeout(timeout)
121110

111+
res.once('aborted', () => {
112+
handleError(Object.assign(new Error('Response aborted'), { code: 'ECONNRESET' }))
113+
})
114+
res.once('error', handleError)
115+
res.once('timeout', () => {
116+
const error = Object.assign(new Error('Response timed out'), { code: 'ETIMEDOUT' })
117+
res.destroy(error)
118+
handleError(error)
119+
})
120+
122121
res.on('data', chunk => {
123122
chunks.push(chunk)
124123
})
125124

126125
res.once('end', () => {
127-
finalize()
128126
const buffer = Buffer.concat(chunks)
129127

130128
if (res.statusCode >= 200 && res.statusCode <= 299) {
131-
const isGzip = res.headers['content-encoding'] === 'gzip'
129+
const contentEncoding = res.headers['content-encoding']
130+
const isGzip = typeof contentEncoding === 'string' && contentEncoding.toLowerCase() === 'gzip'
132131
if (isGzip) {
133132
zlib.gunzip(buffer, (err, result) => {
134133
if (err) {
135134
log.error('Could not gunzip response: %s', err.message)
136-
callback(null, '', res.statusCode, res.headers)
135+
complete(null, '', res.statusCode, res.headers)
137136
} else {
138-
callback(null, result.toString(), res.statusCode, res.headers)
137+
complete(null, result.toString(), res.statusCode, res.headers)
139138
}
140139
})
141140
} else {
142-
callback(null, buffer.toString(), res.statusCode, res.headers)
141+
complete(null, buffer.toString(), res.statusCode, res.headers)
143142
}
144143
} else {
145144
let errorMessage = ''
@@ -160,7 +159,7 @@ function request (data, options, callback) {
160159
const error = new log.NoTransmitError(errorMessage)
161160
error.status = res.statusCode
162161

163-
callback(error, null, res.statusCode, res.headers)
162+
complete(error, null, res.statusCode, res.headers)
164163
}
165164
})
166165
}
@@ -179,30 +178,51 @@ function request (data, options, callback) {
179178

180179
legacyStorage.run({ noop: true }, () => {
181180
let finished = false
181+
let settled = false
182182
const finalize = () => {
183183
if (finished) return
184184
finished = true
185185
activeBufferSize -= options.headers['Content-Length'] ?? 0
186186
}
187187

188-
const req = client.request(options, (res) => onResponse(res, finalize))
188+
/**
189+
* @param {Error | null} error
190+
* @param {string | null} [result]
191+
* @param {number} [statusCode]
192+
* @param {import('node:http').IncomingHttpHeaders} [headers]
193+
*/
194+
const complete = (error, result, statusCode, headers) => {
195+
if (settled) return
196+
settled = true
197+
finalize()
198+
callback(error, result, statusCode, headers)
199+
}
189200

190-
req.once('close', finalize)
191-
req.once('timeout', finalize)
201+
/**
202+
* @param {Error} error
203+
*/
204+
const handleError = (error) => {
205+
if (settled) return
192206

193-
req.once('error', error => {
194-
finalize()
195207
if (options.retry !== false &&
196208
attemptIndex < getMaxAttempts(options) &&
197209
isRetriableNetworkError(error)) {
210+
settled = true
211+
finalize()
198212
// Unref so a pending retry never keeps the host process alive past
199213
// its natural exit point; long-running apps still retry because the
200214
// event loop is held open by their own work.
201215
setTimeout(attempt, getRetryDelay(options, attemptIndex), attemptIndex + 1).unref?.()
202216
} else {
203-
callback(error)
217+
complete(error)
204218
}
205-
})
219+
}
220+
221+
const req = client.request(options, (res) => onResponse(res, complete, handleError))
222+
223+
req.once('close', finalize)
224+
req.once('timeout', finalize)
225+
req.once('error', handleError)
206226

207227
req.setTimeout(timeout, () => {
208228
try {

packages/dd-trace/src/exporters/common/url.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,21 @@
11
'use strict'
22

3+
const net = require('node:net')
4+
35
const { urlToHttpOptions } = require('./url-to-http-options-polyfill')
46

7+
/**
8+
* @param {string} hostname
9+
* @returns {boolean}
10+
*/
11+
function isLoopbackHost (hostname) {
12+
// Gate the 127/8 prefix on an IPv4 literal so names such as 127.example.com cannot pass.
13+
return hostname === 'localhost' ||
14+
hostname === '::1' ||
15+
hostname === '[::1]' ||
16+
(hostname.startsWith('127.') && net.isIPv4(hostname))
17+
}
18+
519
/**
620
* Convert an agent/intake URL into Node http(s) request options.
721
*
@@ -31,4 +45,4 @@ function parseUrl (urlObjOrString) {
3145
return url
3246
}
3347

34-
module.exports = { parseUrl }
48+
module.exports = { isLoopbackHost, parseUrl }

packages/dd-trace/src/feature-registry.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
'use strict'
22

3+
/**
4+
* @typedef {{ enable: (config: import('./config/config-base')) => void, disable: () => void }} FeatureModule
5+
* @typedef {new (tracer: import('./tracer'), config: import('./config/config-base')) => object} FeatureProvider
6+
*/
7+
38
/**
49
* @typedef {object} Feature
510
* @property {string} name
611
* @property {object} noop
7-
* @property {() => object} factory
12+
* @property {() => FeatureModule} factory
813
* @property {(config: import('./config/config-base')) => boolean} isEnabled
9-
* @property {() => Function} provider
10-
* @property {Function} [remoteConfig]
14+
* @property {() => FeatureProvider} provider
15+
* @property {(rc: import('./remote_config'), config: import('./config/config-base'),
16+
* proxy: import('./proxy')) => void} [remoteConfig]
1117
*/
1218

1319
/** @type {{ [name: string]: Feature }} */

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

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
/* eslint-disable no-await-in-loop -- Polls and retries must remain sequential. */
44

55
const { setTimeout: sleep } = require('node:timers/promises')
6-
const { inspect } = require('node:util')
76

87
const request = require('../exporters/common/request')
98
const { getClientLibraryHeaders } = require('../exporters/common/client-library-headers')
@@ -51,9 +50,10 @@ class AgentlessConfigurationSource {
5150
/** @type {string | undefined} */
5251
#etag
5352

54-
#malformedPayloadLogged = false
53+
/** @type {Set<string>} */
54+
#failureWarnings = new Set()
5555

56-
#pollFailureLogged = false
56+
#malformedPayloadLogged = false
5757

5858
/**
5959
* @param {AgentlessSourceConfig} config
@@ -117,7 +117,7 @@ class AgentlessConfigurationSource {
117117

118118
const retryable = response.statusCode === undefined || isRetryableStatus(response.statusCode)
119119
if (!retryable) {
120-
this.#apply(abortController, response)
120+
this.#apply(response)
121121
return
122122
}
123123

@@ -174,11 +174,10 @@ class AgentlessConfigurationSource {
174174
}
175175

176176
/**
177-
* @param {AbortController} abortController
178177
* @param {PollResponse} response
179178
* @returns {void}
180179
*/
181-
#apply (abortController, response) {
180+
#apply (response) {
182181
const statusCode = response.statusCode
183182
if (statusCode === 304) return
184183

@@ -192,10 +191,10 @@ class AgentlessConfigurationSource {
192191
let configuration
193192
try {
194193
configuration = parseConfiguration(response.body)
195-
} catch (error) {
194+
} catch {
196195
if (!this.#malformedPayloadLogged) {
197196
this.#malformedPayloadLogged = true
198-
log.error('Feature Flagging agentless endpoint returned malformed UFC payload: %s', errorMessage(error))
197+
log.error('Feature Flagging agentless endpoint returned malformed UFC payload')
199198
}
200199
return
201200
}
@@ -222,8 +221,11 @@ class AgentlessConfigurationSource {
222221
* @returns {void}
223222
*/
224223
#warnFailure (statusCode, error, attempts) {
225-
if (this.#pollFailureLogged) return
226-
this.#pollFailureLogged = true
224+
const category = statusCode === 401 || statusCode === 403
225+
? 'authentication'
226+
: statusCode ? 'http' : 'request'
227+
if (this.#failureWarnings.has(category)) return
228+
this.#failureWarnings.add(category)
227229

228230
if (statusCode === 401 || statusCode === 403) {
229231
log.warn(
@@ -272,11 +274,7 @@ function parseConfiguration (body) {
272274
!attributes.flags ||
273275
typeof attributes.flags !== 'object' ||
274276
Array.isArray(attributes.flags)) {
275-
throw new Error(
276-
`Expected a Universal Flag Configuration v1 object; received ${
277-
inspect(attributes, { depth: 0, maxStringLength: 0 })
278-
}`
279-
)
277+
throw new Error('Expected a Universal Flag Configuration v1 object')
280278
}
281279

282280
return attributes

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict'
22

3+
const { isLoopbackHost } = require('../exporters/common/url')
34
const log = require('../log')
45

56
const DEFAULT_AGENTLESS_PATH = '/api/v2/feature-flagging/config/rules-based/server'
@@ -66,13 +67,16 @@ function endpoint (config, configuredBaseUrl) {
6667
let url
6768
try {
6869
url = new URL(configured)
69-
} catch (error) {
70-
throw new Error(`Invalid Feature Flagging agentless URL: ${configured}`, { cause: error })
70+
} catch {
71+
throw new Error('Invalid Feature Flagging agentless URL')
7172
}
7273

7374
if (url.protocol !== 'https:' && url.protocol !== 'http:') {
7475
throw new Error('Feature Flagging agentless URL must use HTTP or HTTPS')
7576
}
77+
if (url.protocol === 'http:' && !isLoopbackHost(url.hostname)) {
78+
throw new Error('Feature Flagging agentless URL must use HTTPS unless it targets loopback')
79+
}
7680

7781
if (url.pathname === '' || url.pathname === '/') {
7882
url.pathname = DEFAULT_AGENTLESS_PATH

0 commit comments

Comments
 (0)