Skip to content

Guard OTLP endpoints against scheme-only authorization headers #59315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Sep 8, 2026
Merged

Guard OTLP endpoints against scheme-only authorization headers #59315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e462c5e
Initial plan
Copilot Sep 7, 2026
6b31914
Fix scheme-only OTLP authorization guard
Copilot Sep 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion actions/setup/js/send_otlp_span.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,27 @@ function parseOTLPHeaders(raw) {
return result;
}

const EMPTY_OTLP_AUTHORIZATION_SCHEMES = new Set([

@github-actions github-actions Bot Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/codebase-design] This hardcodes 18 specific auth-scheme names as a blocklist, which is a shallow fix for what's really a structural problem: any header value that is "just a scheme token with no credential" is invalid, regardless of which scheme name is used.

💡 Suggestion: detect structurally instead of enumerating schemes

A scheme-only Authorization/x-sentry-auth value never contains the credential material itself — the credential comes after a space (Scheme <token>) or is the whole value for schemeless auth. Consider testing structure instead of matching a fixed vocabulary, e.g.:

function looksLikeSchemeOnly(value) {
  // A bare token of only letters/digits/hyphens (no whitespace, no other
  // credential-shaped characters) with no separator is almost certainly
  // just an auth-scheme name, not a filled-in credential.
  return /^[A-Za-z][A-Za-z0-9-]*$/.test(value);
}

This avoids needing to keep EMPTY_OTLP_AUTHORIZATION_SCHEMES in sync with every current and future auth scheme (custom/vendor schemes like AWS4-HMAC-SHA256, Signature, hawk, etc. are not in the list and would silently slip past the guard this PR adds). It also removes ~20 lines of enumeration that need justifying/maintaining.

If the fixed list is intentionally conservative (to avoid false positives on legitimate single-word tokens), it'd help to note that tradeoff in a comment near the Set so future readers understand why a structural check wasn't used.

@copilot please address this.

"api-key",
"apikey",
"basic",
"bearer",
"concealed",
"digest",
"dpop",
"dsn",
"gnap",
"hoba",
"mutual",
"negotiate",
"oauth",
"privatetoken",
"scram-sha-1",
"scram-sha-256",
"sentry",
"vapid",
]);

/**
* @param {string} raw
* @returns {boolean}
Expand All @@ -881,7 +902,7 @@ function hasEmptyOTLPAuthorizationHeader(raw) {
const headers = parseOTLPHeaders(raw);
return Object.entries(headers).some(([key, value]) => {
const normalizedKey = key.toLowerCase();
return (normalizedKey === "authorization" || normalizedKey === "x-sentry-auth") && value === "";
return (normalizedKey === "authorization" || normalizedKey === "x-sentry-auth") && (value === "" || EMPTY_OTLP_AUTHORIZATION_SCHEMES.has(value.toLowerCase()));

@github-actions github-actions Bot Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This drops any Authorization/x-sentry-auth value that happens to equal a token in a hard-coded scheme list, but that is not the same thing as proving the credentials are missing. A real secret can legitimately be ApiKey, DSN, or another short opaque value, so this heuristic can silently disable telemetry for valid configurations.

💡 Tighten this to detect structurally empty credentials instead of matching literal secret values

Right now the runtime is making a semantic decision from the secret payload alone:

EMPTY_OTLP_AUTHORIZATION_SCHEMES.has(value.toLowerCase())

That is safe only if the format contract guarantees those bare values are invalid for every supported backend, and the surrounding code/docs do not establish that. If the goal is specifically to catch expressions like Authorization=Bearer after secret expansion, validate the structure that proves "scheme with no credential" instead of rejecting any literal value equal to a known token.

For example, constrain the check to formats you actually own, or move the normalization earlier so the compiler/runtime can distinguish scheme + missing secret from an opaque secret value:

const authMatch = value.match(/^(\S+)\s+(.+)$/);
if (authMatch && EMPTY_OTLP_AUTHORIZATION_SCHEMES.has(authMatch[1].toLowerCase()) && authMatch[2].trim() === "") {
  return true;
}

That keeps the guard focused on incomplete credentials without inventing invalidity for otherwise non-empty secrets.

@github-actions github-actions Bot Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The Digest%20%20 test case only passes because parseOTLPHeaders trims the decoded value down to "digest" before this check runs — that trimming behavior isn't obvious from this line and isn't asserted directly. A reader modifying parseOTLPHeaders trimming could silently break this guard without any test failing here.

💡 Suggestion

Consider adding a unit test directly on hasEmptyOTLPAuthorizationHeader/parseOTLPHeaders (if exported for testing) asserting that internal/trailing whitespace around a scheme name is trimmed prior to the Set.has() lookup, so the dependency between these two functions is explicit rather than implicit through an end-to-end parseOTLPEndpoints test.

@copilot please address this.

});
}

Expand Down
11 changes: 11 additions & 0 deletions actions/setup/js/send_otlp_span.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6705,6 +6705,17 @@ describe("parseOTLPEndpoints", () => {
expect(parseOTLPEndpoints()).toEqual([]);
});

it.each(["Authorization=ApiKey", "Authorization=Bearer", "Authorization=Digest%20%20", "x-sentry-auth=ApiKey", "x-sentry-auth=DSN"])("drops an endpoint when its %s header has an auth scheme without credentials", headers => {
process.env.GH_AW_OTLP_ENDPOINTS = JSON.stringify([{ url: "https://traces.example.com:4317", headers }]);
expect(parseOTLPEndpoints()).toEqual([]);
});

it("keeps an endpoint when its authorization header has credentials", () => {
const endpoint = { url: "https://traces.example.com:4317", headers: "Authorization=ApiKey secret-token" };
process.env.GH_AW_OTLP_ENDPOINTS = JSON.stringify([endpoint]);
expect(parseOTLPEndpoints()).toEqual([endpoint]);
});

it("keeps an endpoint when an unrelated header is empty", () => {
process.env.GH_AW_OTLP_ENDPOINTS = JSON.stringify([{ url: "https://traces.example.com:4317", headers: "X-Tenant=" }]);
expect(parseOTLPEndpoints()).toEqual([{ url: "https://traces.example.com:4317", headers: "X-Tenant=" }]);
Expand Down
2 changes: 2 additions & 0 deletions docs/src/content/docs/reference/open-telemetry.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ observability:

Once configured, gh-aw exports built-in workflow spans such as setup and conclusion events to the configured OTLP backend.

Use the bare secret expression for authorization headers, as shown above, rather than adding an authentication scheme prefix. For Sentry endpoints, gh-aw automatically rewrites `Authorization` to `x-sentry-auth`, so no prefix is needed.

### Organization-wide defaults

When a workflow does not configure `observability.otlp` (in its own frontmatter or through an import), the compiler falls back to a default OTLP configuration read from the GitHub Actions environment:
Expand Down
Loading