Skip to content

Commit

Permalink
feat: sanitize http.url attribute values
Browse files Browse the repository at this point in the history
  • Loading branch information
@daniel-white
daniel-white committed Jan 6, 2023
1 parent 2dcc898 commit daec69f
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

* feat(api): add `getActiveBaggage` API [#3385](https://github.com/open-telemetry/opentelemetry-js/pull/3385)
* feat(instrumentation-grpc): set net.peer.name and net.peer.port on client spans [#3430](https://github.com/open-telemetry/opentelemetry-js/pull/3430)
* feat(opentelemetry-core): sanitize `http.url` attributes to remove usernames and passwords [#3487](https://github.com/open-telemetry/opentelemetry-js/pull/3487) @daniel-white

### :bug: (Bug Fix)

Expand Down
91 changes: 75 additions & 16 deletions packages/opentelemetry-core/src/common/attributes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,56 @@
* limitations under the License.
*/

import { diag, SpanAttributeValue, SpanAttributes } from '@opentelemetry/api';
import { diag, AttributeValue, Attributes } from '@opentelemetry/api';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';

const valueSanitizers: ReadonlyMap<
string,
(val: AttributeValue) => AttributeValue | undefined
> = new Map([[SemanticAttributes.HTTP_URL, sanitizeHttpUrl]]);

export function sanitizeAttribute(
key: string,
val: AttributeValue
): AttributeValue | undefined {
let out: AttributeValue | undefined = undefined;

if (!isAttributeKey(key)) {
diag.warn(`Invalid attribute key: ${key}`);
return out;
}
if (!isAttributeValue(val)) {
diag.warn(`Invalid attribute value set for key: ${key}`);
return out;
}

let copyVal: AttributeValue;

if (Array.isArray(val)) {
copyVal = val.slice();
} else {
copyVal = val;
}

const valueSanitizer = valueSanitizers.get(key);
if (typeof valueSanitizer === 'undefined') {
return copyVal;
}

export function sanitizeAttributes(attributes: unknown): SpanAttributes {
const out: SpanAttributes = {};
return valueSanitizer(copyVal);
}

export function sanitizeAttributes(attributes: unknown): Attributes {
const out: Attributes = {};

if (typeof attributes !== 'object' || attributes == null) {
return out;
}

for (const [key, val] of Object.entries(attributes)) {
if (!isAttributeKey(key)) {
diag.warn(`Invalid attribute key: ${key}`);
continue;
}
if (!isAttributeValue(val)) {
diag.warn(`Invalid attribute value set for key: ${key}`);
continue;
}
if (Array.isArray(val)) {
out[key] = val.slice();
} else {
out[key] = val;
const sanitizedVal = sanitizeAttribute(key, val);
if (typeof sanitizedVal !== 'undefined') {
out[key] = sanitizedVal;
}
}

Expand All @@ -46,7 +74,7 @@ export function isAttributeKey(key: unknown): key is string {
return typeof key === 'string' && key.length > 0;
}

export function isAttributeValue(val: unknown): val is SpanAttributeValue {
export function isAttributeValue(val: unknown): val is AttributeValue {
if (val == null) {
return true;
}
Expand Down Expand Up @@ -94,3 +122,34 @@ function isValidPrimitiveAttributeValue(val: unknown): boolean {

return false;
}

function sanitizeHttpUrl(val: AttributeValue): AttributeValue | undefined {
let out: AttributeValue | undefined;

if (typeof val !== 'string') {
diag.warn(
`Invalid attribute value set for key: ${
SemanticAttributes.HTTP_URL
}. Unable to sanitize ${Array.isArray(val) ? 'array' : typeof val} value.`
);

return val;
}

try {
const valUrl = new URL(val);

valUrl.username = '';
valUrl.password = '';

out = valUrl.toString();
} catch (err: unknown) {
diag.warn(
`Invalid attribute value set for key: ${SemanticAttributes.HTTP_URL}. Unable to sanitize invalid URL.`
);

out = val;
}

return out;
}
88 changes: 88 additions & 0 deletions packages/opentelemetry-core/test/common/attributes.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,28 @@
* limitations under the License.
*/

import { diag } from '@opentelemetry/api';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import * as assert from 'assert';
import * as sinon from 'sinon';
import {
isAttributeValue,
sanitizeAttribute,
sanitizeAttributes,
} from '../../src/common/attributes';

describe('attributes', () => {
const warnStub = sinon.fake();

beforeEach(() => {
diag.setLogger({
warn: warnStub,
} as any);

// diag.warn is used when the logger is set
warnStub.resetHistory();
});

describe('#isAttributeValue', () => {
it('should allow primitive values', () => {
assert.ok(isAttributeValue(0));
Expand Down Expand Up @@ -102,5 +117,78 @@ describe('attributes', () => {
assert.ok(Array.isArray(attributes.arr));
assert.strictEqual(attributes.arr[0], 'unmodified');
});

describe('value sanitizers', () => {
describe('http.url', () => {
it('should remove username and password from the url', () => {
const out = sanitizeAttribute(
SemanticAttributes.HTTP_URL,
'http://user:pass@host:9000/path?query#fragment'
);

assert.strictEqual(out, 'http://host:9000/path?query#fragment');
assert.ok(warnStub.notCalled, 'should not log warning');
});

it('should return the same url', () => {
const out = sanitizeAttribute(
SemanticAttributes.HTTP_URL,
'http://host:9000/path?query#fragment'
);

assert.strictEqual(out, 'http://host:9000/path?query#fragment');
assert.ok(warnStub.notCalled, 'should not log warning');
});

it('should return the input string when the value is not a valid url', () => {
const out = sanitizeAttribute(
SemanticAttributes.HTTP_URL,
'invalid url'
);

assert.strictEqual(out, 'invalid url');
assert.ok(
warnStub.calledWithExactly(
`Invalid attribute value set for key: ${SemanticAttributes.HTTP_URL}. Unable to sanitize invalid URL.`
)
);
});

it('should return the input when the value is a number', () => {
const out = sanitizeAttribute(SemanticAttributes.HTTP_URL, 27);

assert.strictEqual(out, 27);
assert.ok(
warnStub.calledWithExactly(
`Invalid attribute value set for key: ${SemanticAttributes.HTTP_URL}. Unable to sanitize number value.`
)
);
});

it('should return the input when the value is boolean', () => {
const out = sanitizeAttribute(SemanticAttributes.HTTP_URL, false);

assert.strictEqual(out, false);
assert.ok(
warnStub.calledWithExactly(
`Invalid attribute value set for key: ${SemanticAttributes.HTTP_URL}. Unable to sanitize boolean value.`
)
);
});

it('should return the input when an array is supplied', () => {
const out = sanitizeAttribute(SemanticAttributes.HTTP_URL, [
'http://host/path?query#fragment',
]);

assert.deepStrictEqual(out, ['http://host/path?query#fragment']);
assert.ok(
warnStub.calledWithExactly(
`Invalid attribute value set for key: ${SemanticAttributes.HTTP_URL}. Unable to sanitize array value.`
)
);
});
});
});
});
});
22 changes: 14 additions & 8 deletions packages/opentelemetry-sdk-trace-base/src/Span.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
*/

import * as api from '@opentelemetry/api';
import { Context, HrTime, SpanAttributeValue } from '@opentelemetry/api';
import { Context, HrTime, AttributeValue } from '@opentelemetry/api';
import {
Clock,
hrTimeDuration,
InstrumentationLibrary,
isAttributeValue,
isTimeInput,
otperformance,
sanitizeAttribute,
sanitizeAttributes,
timeInputToHrTime,
} from '@opentelemetry/core';
Expand All @@ -44,7 +45,7 @@ export class Span implements api.Span, ReadableSpan {
private readonly _spanContext: api.SpanContext;
readonly kind: api.SpanKind;
readonly parentSpanId?: string;
readonly attributes: api.SpanAttributes = {};
readonly attributes: api.Attributes = {};
readonly links: api.Link[] = [];
readonly events: TimedEvent[] = [];
readonly startTime: api.HrTime;
Expand Down Expand Up @@ -98,13 +99,14 @@ export class Span implements api.Span, ReadableSpan {
return this._spanContext;
}

setAttribute(key: string, value?: SpanAttributeValue): this;
setAttribute(key: string, value?: AttributeValue): this;
setAttribute(key: string, value: unknown): this {
if (value == null || this._isSpanEnded()) return this;
if (key.length === 0) {
api.diag.warn(`Invalid attribute key: ${key}`);
return this;
}

if (!isAttributeValue(value)) {
api.diag.warn(`Invalid attribute value set for key: ${key}`);
return this;
Expand All @@ -117,11 +119,15 @@ export class Span implements api.Span, ReadableSpan {
) {
return this;
}
this.attributes[key] = this._truncateToSize(value);

const sanitizedValue = sanitizeAttribute(key, value);
if (typeof sanitizedValue !== 'undefined') {
this.attributes[key] = this._truncateToSize(sanitizedValue);
}
return this;
}

setAttributes(attributes: api.SpanAttributes): this {
setAttributes(attributes: api.Attributes): this {
for (const [k, v] of Object.entries(attributes)) {
this.setAttribute(k, v);
}
Expand All @@ -137,7 +143,7 @@ export class Span implements api.Span, ReadableSpan {
*/
addEvent(
name: string,
attributesOrStartTime?: api.SpanAttributes | api.TimeInput,
attributesOrStartTime?: api.Attributes | api.TimeInput,
startTime?: api.TimeInput
): this {
if (this._isSpanEnded()) return this;
Expand Down Expand Up @@ -211,7 +217,7 @@ export class Span implements api.Span, ReadableSpan {
exception: api.Exception,
time: api.TimeInput = this._clock.now()
): void {
const attributes: api.SpanAttributes = {};
const attributes: api.Attributes = {};
if (typeof exception === 'string') {
attributes[SemanticAttributes.EXCEPTION_MESSAGE] = exception;
} else if (exception) {
Expand Down Expand Up @@ -279,7 +285,7 @@ export class Span implements api.Span, ReadableSpan {
* @param value Attribute value
* @returns truncated attribute value if required, otherwise same value
*/
private _truncateToSize(value: SpanAttributeValue): SpanAttributeValue {
private _truncateToSize(value: AttributeValue): AttributeValue {
const limit = this._attributeValueLengthLimit;
// Check limit
if (limit <= 0) {
Expand Down
15 changes: 11 additions & 4 deletions packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import * as assert from 'assert';
import * as sinon from 'sinon';
import { BasicTracerProvider, Span, SpanProcessor } from '../../src';
import { invalidAttributes, validAttributes } from './util';
import {
invalidAttributes,
sanitizedValidAttributes,
validAttributes,
} from './util';

const performanceTimeOrigin: HrTime = [1, 1];

Expand Down Expand Up @@ -260,7 +264,7 @@ describe('Span', () => {
span.setAttribute(k, v as unknown as SpanAttributeValue);
}

assert.deepStrictEqual(span.attributes, validAttributes);
assert.deepStrictEqual(span.attributes, sanitizedValidAttributes);
});

it('should be able to overwrite attributes', () => {
Expand Down Expand Up @@ -708,7 +712,7 @@ describe('Span', () => {
span.setAttributes(validAttributes);
span.setAttributes(invalidAttributes as unknown as SpanAttributes);

assert.deepStrictEqual(span.attributes, validAttributes);
assert.deepStrictEqual(span.attributes, sanitizedValidAttributes);
});
});

Expand Down Expand Up @@ -742,7 +746,10 @@ describe('Span', () => {

assert.strictEqual(span.events.length, 1);
assert.deepStrictEqual(span.events[0].name, 'rev');
assert.deepStrictEqual(span.events[0].attributes, validAttributes);
assert.deepStrictEqual(
span.events[0].attributes,
sanitizedValidAttributes
);
});
});

Expand Down
8 changes: 8 additions & 0 deletions packages/opentelemetry-sdk-trace-base/test/common/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,21 @@
* limitations under the License.
*/

import { SemanticAttributes } from '@opentelemetry/semantic-conventions';

export const validAttributes = {
string: 'string',
number: 0,
bool: true,
'array<string>': ['str1', 'str2'],
'array<number>': [1, 2],
'array<bool>': [true, false],
[SemanticAttributes.HTTP_URL]: 'http://user:pass@host:port/path?query',
};

export const sanitizedValidAttributes = {
...validAttributes,
[SemanticAttributes.HTTP_URL]: 'http://host:port/path?query',
};

export const invalidAttributes = {
Expand Down

0 comments on commit daec69f

Please sign in to comment.