Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/otlp-metadata-attributes.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions .changeset/otlp-secret-attribute-lint.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# ADR-70860: Resolve Root Workflow Metadata References in OTLP Attributes at Compile Time

**Date**: 2026-10-22
**Status**: Draft
**Deciders**: pelikhan, Copilot coding agent

---

### Context

Workflows can declare a top-level `metadata` map, such as `version`, `category`, and `owner`. They can also declare custom OTLP span attributes (`observability.otlp.attributes`) and resource attributes (`observability.otlp.resource-attributes`). These OTLP maps are often defined once in a shared import, for example `shared/otlp.md`, and reused by many root workflows. Before this change, there was no way for those shared OTLP maps to pick up the importing workflow's metadata (issue #70377). Authors had to copy values by hand or pass them through import inputs. Separately, the parser's import merge (`mergeObservabilityConfigs`) kept span `attributes` but silently dropped imported `resource-attributes`. Any solution has to keep metadata from being exported unless the author opts in, and it must not open a path for GitHub Actions expression injection.

### Decision

We will support whole-value `${{ github.aw.metadata.<key> }}` references in both OTLP custom attribute maps. The compiler resolves them in `resolveOTLPMetadataReferences`, which runs right after imported observability configuration is merged. Values always come from the **root** workflow's top-level `metadata` map, never from the import's own metadata. Resolution is strict. The reference must be the entire attribute value and the key must be a simple identifier. Compilation fails if the key is missing, if the root workflow has no metadata, if the metadata value is not a string, or if it contains `${{` or `}}`. We will also merge imported `resource-attributes` across imports using the same first-occurrence-wins rule already used for span attributes, with the root workflow still taking precedence.

### Alternatives Considered

#### Alternative 1: Pass metadata values through import inputs

Shared OTLP imports could declare inputs that the root workflow fills with its metadata values. This approach uses existing machinery, which is why it was considered. It was rejected because every root workflow would have to repeat its metadata in an `imports` input block. That duplication is exactly the problem issue #70377 describes.

#### Alternative 2: Automatically export all metadata as OTLP attributes

The compiler could copy every `metadata` entry into the OTLP attribute maps without any references. This needs no new syntax. It was rejected because it would export data without anyone asking for it, and metadata may contain values that should not leave the workflow. The documentation added in this PR states that "Metadata is not exported automatically" and that "only explicitly selected values are exported".

#### Alternative 3: Resolve references at runtime as GitHub Actions expressions

The references could be passed through for GitHub Actions or the runtime to expand. This was rejected because `github.aw.metadata` is not a real Actions context. Resolving at runtime would also put metadata text into an expression-evaluation path. Resolving at compile time, and rejecting values that contain expression delimiters, keeps the compiled lock file static and closes off expression injection.

### Consequences

#### Positive
- Shared OTLP configuration can label telemetry with each importing workflow's own metadata, without duplicating values or passing them through import inputs.
- Strict validation (whole-value only, simple keys, string values, no `${{`/`}}` in values) turns mistakes into compile errors and prevents expression injection through metadata.
- Imported `resource-attributes` are no longer dropped silently. Span and resource attributes now share one merge helper (`mergeOTLPStringMapFirstWins`).

#### Negative
- The `${{ ... }}` syntax looks like a GitHub Actions expression but is a compiler-only pseudo-context. Authors may wrongly expect other Actions contexts or embedded interpolation to work there.
- Embedded references (for example `"v${{ github.aw.metadata.version }}"`), nested selectors, and non-string metadata are not supported. These limits may need to be relaxed later.
- The parser's attribute extraction is still a local copy of the workflow package's logic, kept separate to avoid a circular import. This PR widens that duplicated code to cover `resource-attributes`, so there is more to keep in sync.

#### Neutral
- Resolution always uses the root workflow's scope, even when the reference appears in an imported file. A `metadata` map inside the import itself is ignored for this purpose.
- Resolution runs inside `mergeImportedWorkflowConfiguration`, so it depends on running after imported observability is merged and before environment variables are built from the OTLP maps.

---

*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*
3 changes: 3 additions & 0 deletions docs/src/content/docs/reference/frontmatter.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ metadata:
Keys must be 1–64 characters; values are string-only, up to 1024 characters.
`metadata.docs`, when present, must be an absolute HTTPS URL. The compiler preserves
it in generated lock-file metadata without fetching it or changing workflow execution.
To explicitly reuse a metadata value in OTLP custom span or resource attributes, use
a whole-value `${{ github.aw.metadata.<key> }}` reference; it resolves against this
root workflow's metadata at compile time. Metadata is not exported automatically.

### Trigger Events (`on:`)

Expand Down
2 changes: 2 additions & 0 deletions docs/src/content/docs/reference/imports.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ imports:

`shared/reporting-otlp.md` combines `shared/reporting.md` and `shared/otlp.md` for telemetry-enabled reporting workflows.

Shared OTLP configuration can use whole-value `${{ github.aw.metadata.<key> }}` references in `observability.otlp.attributes` and `observability.otlp.resource-attributes`. These resolve at compile time against each importing root workflow's `metadata` map, so callers do not need to pass the values through import inputs. Only simple, existing metadata keys can be referenced, and only explicitly selected values are exported.

## Import Schema (`import-schema`)

Use `import-schema` to declare a typed parameter contract. Callers pass values via `with`; the compiler validates them and substitutes them into the shared file's frontmatter and body before processing.
Expand Down
4 changes: 4 additions & 0 deletions docs/src/content/docs/reference/open-telemetry.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,13 @@ observability:
deployment.environment: production
langfuse.session.id: "{{ gh-aw.episode.id }}"
langfuse.user.id: "{{ github.actor }}"
catalogue.version: "${{ github.aw.metadata.version }}"
```

Values are plain strings. Template expressions such as `{{ gh-aw.episode.id }}` or `{{ github.actor }}` are resolved from attributes already computed for the span. Empty values are omitted. Values of at least 4 characters are masked in runner logs; shorter values are not masked.

Whole-value references in the form `${{ github.aw.metadata.<key> }}` resolve at compile time from the root workflow's top-level `metadata` map. They work in either custom attribute map, including in shared imported OTLP configuration, and support simple metadata keys such as `version`, `category`, and `owner`. The reference must occupy the entire attribute value; embedded references, nested selectors, missing metadata keys, and metadata values containing GitHub Actions expression delimiters are compile-time errors. Metadata remains string-only, and only explicitly referenced values are exported.

## Custom resource attributes

`observability.otlp.resource-attributes` appends additional resource attributes to the standard gh-aw and GitHub resource attribute set:
Expand All @@ -132,6 +135,7 @@ observability:
resource-attributes:
service.namespace: platform-automation
deployment.environment: ${{ github.ref_name }}
catalogue.category: "${{ github.aw.metadata.category }}"
```

Use static strings or GitHub Actions expressions. Do **not** use `secrets.*` or `vars.*` values in `resource-attributes` because resource attributes are exported to external observability backends and are not treated as secret values.
Expand Down
3 changes: 3 additions & 0 deletions docs/src/content/docs/reference/workflow-linters.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ The compiler threat IDs reference the [Compiler Threat Detection Specification](
| `template-injection` | `CTR-006` | AW, Actions | Known attacker-controlled expressions are not interpolated into executable `run` or `actions/github-script` code. Use environment bindings and quoted shell expansions or `process.env` instead. |
| `workflow-run-scope` | `CTR-021` | AW, Actions | `workflow_run` selects workflows and has a positive branch scope, not just wildcard, negated-only, or empty branch filters. |
| `secret-leak` | `CTR-017` | AW, Actions | Symbolic secrets do not reach modeled disclosure sinks. AW-only checks report missing generated execution coverage. |
| `otlp-secret-leak` | `CTR-017` | AW, Actions | Secret-tainted expressions are not exported through OTLP span/resource attributes. Actions AST analysis follows modeled environment bindings; AW-only analysis checks direct references. Does not detect hard-coded credentials or arbitrary runtime values. |
| `permission-scope` | `CTR-001` | AW, Actions | Explicit permissions avoid blanket `write-all` grants; OIDC does not use invalid `id-token: read`. Scoped writes and `id-token: write` are not automatically violations. |
| `version-check` | `CTR-018` | AW | Authored source does not disable `check-for-updates`. |
| `conditional-import` | `CTR-020` | AW | Configuration import entries do not contain `if`, even if false or null. Runtime prompt conditionals are unaffected. |
Expand Down Expand Up @@ -65,6 +66,8 @@ AW checks inspect explicit execution blocks, including `safe-outputs.steps`, cus

This is not complete zizmor parity. Creator-verification findings such as [#50015](https://github.com/github/gh-aw/pull/50015) require external provenance data; static ASTs do not establish creator trust. Existing specialized trigger and permission rules remain separate, and zizmor ignore comments are not AST-rule suppressions.

`otlp-secret-leak` reports direct secret references in authored `observability.otlp.attributes` and `resource-attributes`, and follows modeled environment flows to Actions `GH_AW_OTLP_ATTRIBUTES` and `OTEL_RESOURCE_ATTRIBUTES` bindings. Diagnostics identify only the configuration path, never the value. The check is advisory; it does not inspect literal credentials, arbitrary runtime-generated values, or collector trust.

## Severity and source policy

Action pinning, workflow-run branch scope, blanket write permissions, disabled version checking, private-to-public opt-outs, enabled Node install hooks, and bare network wildcard allowances are errors for an AW document with strict mode enabled (the AW default), and warnings for explicit non-strict AW. Native Actions policy-sensitive findings are warnings; AW-only rules skip native Actions without an AW document. Conditional imports, bare label-allowlist wildcards, invalid OIDC read grants, missing or malformed `workflow_run.workflows`, invalid explicit timeouts, and direct template injection are errors. Unconstrained wildcard push targets remain warnings even in strict mode. The linter does not reconstruct CLI/repository strict overrides, load imported configurations, infer missing permissions, or determine whether each repository-write grant is authorized.
Expand Down
43 changes: 31 additions & 12 deletions pkg/parser/import_observability.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ func extractOTLPEndpointsFromObsMap(obs map[string]any) []observabilityImportEnd
result = append(result, entry)
}
case map[string]any:
if url, _ := ep["url"].(string); url != "" {
url, ok := ep["url"].(string)
if ok && url != "" {
entry := observabilityImportEndpoint{URL: url}
if h, hasH := ep["headers"]; hasH {
entry.Headers = h
Expand All @@ -59,8 +60,8 @@ func extractOTLPEndpointsFromObsMap(obs map[string]any) []observabilityImportEnd
if !ok {
continue
}
url, _ := itemMap["url"].(string)
if url == "" {
url, ok := itemMap["url"].(string)
if !ok || url == "" {
continue
}
entry := observabilityImportEndpoint{URL: url}
Expand All @@ -76,13 +77,15 @@ func extractOTLPEndpointsFromObsMap(obs map[string]any) []observabilityImportEnd
// mergeObservabilityConfigs takes a slice of observability config JSON strings (one per
// import), extracts all OTLP endpoint entries from each (supporting string, object, and
// array forms), deduplicates by URL (first occurrence wins), and returns a single merged
// observability JSON string with all endpoints expressed as an array. Custom OTLP
// attributes are also merged across imports (first occurrence wins per key).
// observability JSON string with all endpoints expressed as an array. Custom OTLP
// span and resource attributes are also merged across imports (first occurrence wins
// per key).
// Returns "" when no valid endpoints or attributes are found.
func mergeObservabilityConfigs(configs []string) string {
seen := make(map[string]struct{})
var allEndpoints []observabilityImportEndpoint
mergedAttrs := make(map[string]string)
mergedResourceAttrs := make(map[string]string)
var mergedGitHubApp map[string]any

for i, cfgJSON := range configs {
Expand All @@ -100,17 +103,14 @@ func mergeObservabilityConfigs(configs []string) string {
allEndpoints = append(allEndpoints, e)
}
}
for k, v := range extractOTLPAttributesFromObsMap(obs) {
if _, exists := mergedAttrs[k]; !exists {
mergedAttrs[k] = v
}
}
mergeOTLPStringMapFirstWins(mergedAttrs, extractOTLPAttributesFromObsMap(obs))
mergeOTLPStringMapFirstWins(mergedResourceAttrs, extractOTLPResourceAttributesFromObsMap(obs))
if mergedGitHubApp == nil {
mergedGitHubApp = extractOTLPGitHubAppFromObsMap(obs)
}
}

if len(allEndpoints) == 0 && len(mergedAttrs) == 0 && mergedGitHubApp == nil {
if len(allEndpoints) == 0 && len(mergedAttrs) == 0 && len(mergedResourceAttrs) == 0 && mergedGitHubApp == nil {
return ""
}

Expand All @@ -124,6 +124,9 @@ func mergeObservabilityConfigs(configs []string) string {
if len(mergedAttrs) > 0 {
otlpMap["attributes"] = mergedAttrs
}
if len(mergedResourceAttrs) > 0 {
otlpMap["resource-attributes"] = mergedResourceAttrs
}
if mergedGitHubApp != nil {
otlpMap["github-app"] = mergedGitHubApp
}
Expand All @@ -136,6 +139,14 @@ func mergeObservabilityConfigs(configs []string) string {
return string(b)
}

func mergeOTLPStringMapFirstWins(destination, source map[string]string) {
for key, value := range source {
if _, exists := destination[key]; !exists {
destination[key] = value
}
}
}

func extractOTLPGitHubAppFromObsMap(obs map[string]any) map[string]any {
if obs == nil {
return nil
Expand Down Expand Up @@ -171,6 +182,14 @@ func extractOTLPGitHubAppFromObsMap(obs map[string]any) map[string]any {
// import the workflow package (circular-dependency risk), so the helper lives
// here as a local copy. Both implementations must stay in sync.
func extractOTLPAttributesFromObsMap(obs map[string]any) map[string]string {
return extractOTLPStringMapFromObsMap(obs, "attributes")
}

func extractOTLPResourceAttributesFromObsMap(obs map[string]any) map[string]string {
return extractOTLPStringMapFromObsMap(obs, "resource-attributes")
}

func extractOTLPStringMapFromObsMap(obs map[string]any, fieldName string) map[string]string {
if obs == nil {
return nil
}
Expand All @@ -182,7 +201,7 @@ func extractOTLPAttributesFromObsMap(obs map[string]any) map[string]string {
if !ok {
return nil
}
attrsAny, ok := otlpMap["attributes"]
attrsAny, ok := otlpMap[fieldName]
if !ok {
return nil
}
Expand Down
21 changes: 21 additions & 0 deletions pkg/parser/import_observability_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package parser

import (
"encoding/json"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestExtractOTLPAttributesFromObsMap verifies extractOTLPAttributesFromObsMap
Expand Down Expand Up @@ -102,3 +104,22 @@ func TestExtractOTLPAttributesFromObsMap(t *testing.T) {
})
}
}

func TestMergeObservabilityConfigsIncludesResourceAttributes(t *testing.T) {
configs := []string{
`{"otlp":{"resource-attributes":{"shared.key":"first","first.only":"one"}}}`,
`{"otlp":{"resource-attributes":{"shared.key":"second","second.only":"two"}}}`,
}

merged := mergeObservabilityConfigs(configs)
require.NotEmpty(t, merged)

var result map[string]any
require.NoError(t, json.Unmarshal([]byte(merged), &result))
otlp := result["otlp"].(map[string]any)
assert.Equal(t, map[string]any{
"shared.key": "first",
"first.only": "one",
"second.only": "two",
}, otlp["resource-attributes"])
}
3 changes: 3 additions & 0 deletions pkg/workflow/compiler_orchestrator_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,9 @@ func (c *Compiler) mergeImportedWorkflowConfiguration(ctx *workflowBuildContext)
ctx.workflowData.ConcurrencyJobDiscriminator = ctx.engineSetup.importsResult.MergedJobDiscriminator
}
c.mergeImportedObservability(ctx.workflowData, ctx.engineSetup.importsResult.MergedObservability)
if err := resolveOTLPMetadataReferences(ctx.workflowData); err != nil {
return formatCompilerError(ctx.cleanPath, "error", err.Error(), err)
}
if err := c.mergeWorkflowEnv(ctx.frontmatter.Frontmatter, ctx.workflowData, ctx.engineSetup.importsResult); err != nil {
return formatCompilerError(ctx.cleanPath, "error", err.Error(), err)
}
Expand Down
43 changes: 43 additions & 0 deletions pkg/workflow/compiler_orchestrator_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,49 @@ func TestMergeImportedObservability_MergesResourceAttributesWithMainPrecedence(t
}, otlp["resource-attributes"])
}

func TestMergeImportedObservabilityResolvesRootMetadataReferences(t *testing.T) {
importedObsJSON, err := json.Marshal(map[string]any{
"metadata": map[string]any{"version": "import-version"},
"otlp": map[string]any{
"attributes": map[string]any{
"catalogue.version": "${{ github.aw.metadata.version }}",
},
"resource-attributes": map[string]any{
"catalogue.category": "${{ github.aw.metadata.category }}",
},
},
})
require.NoError(t, err)

workflowData := &WorkflowData{
RawFrontmatter: map[string]any{
"metadata": map[string]any{
"version": "root-version",
"category": "automation",
},
},
}

ctx := &workflowBuildContext{
cleanPath: "workflow.md",
frontmatter: &parser.FrontmatterResult{Frontmatter: workflowData.RawFrontmatter},
engineSetup: &engineSetupResult{
importsResult: &parser.ImportsResult{MergedObservability: string(importedObsJSON)},
},
workflowData: workflowData,
}
require.NoError(t, NewCompiler().mergeImportedWorkflowConfiguration(ctx))

obs := workflowData.RawFrontmatter["observability"].(map[string]any)
otlp := obs["otlp"].(map[string]any)
assert.Equal(t, map[string]any{"catalogue.version": "root-version"}, otlp["attributes"])
assert.Equal(t, map[string]any{"catalogue.category": "automation"}, otlp["resource-attributes"])
assert.Equal(t, map[string]string{"catalogue.version": "root-version"}, collectOTLPCustomAttributes(workflowData.RawFrontmatter))
assert.Equal(t, map[string]string{"catalogue.category": "automation"}, collectOTLPResourceAttributes(workflowData.RawFrontmatter))
assert.Contains(t, workflowData.Env, `"catalogue.version":"root-version"`)
assert.Contains(t, workflowData.Env, "catalogue.category=automation")
}

func TestBuildMergedEnvSources_MainWorkflowWins(t *testing.T) {
mergedEnv := map[string]any{
"MAIN_ONLY": "1",
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/lint/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ func sortDiagnostics(diagnostics []Diagnostic) {
func Default() *Runner {
return &Runner{rules: []Rule{
SecretLeakRule(),
OTLPSecretLeakRule(),
ActionPinRule(),
JobTimeoutRule(),
NeedsConsistencyRule(),
Expand Down
2 changes: 1 addition & 1 deletion pkg/workflow/lint/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestRuleRegistrationAndSelection(t *testing.T) {
ruleIDs = append(ruleIDs, rule.ID)
}
assert.ElementsMatch(t, []string{
"secret-leak", "action-pin", "job-timeout", "needs-consistency", "template-injection", "workflow-run-scope",
"secret-leak", "otlp-secret-leak", "action-pin", "job-timeout", "needs-consistency", "template-injection", "workflow-run-scope",
"permission-scope", "version-check", "conditional-import", "private-to-public",
"install-scripts", "allowed-labels", "network-scope", "push-target",
"tool-scope", "sandbox-config", "image-argument", "checkout-mode", "fork-association",
Expand Down
Loading
Loading