Skip to content

feat: add step.auth_validate pipeline step#190

Merged
intel352 merged 3 commits into
mainfrom
copilot/add-auth-validation-step
Feb 27, 2026
Merged

feat: add step.auth_validate pipeline step#190
intel352 merged 3 commits into
mainfrom
copilot/add-auth-validation-step

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 27, 2026

Adds step.auth_validate — a declarative pipeline step that validates ****** against a registered AuthProvider module and outputs the claims returned by the provider into the pipeline context.

Changes

  • module/pipeline_step_auth_validate.go — Step implementation using existing AuthProvider interface and resolveBodyFrom for dot-path token extraction. Writes 401 JSON to HTTP response writer when available, sets Stop: true on all failure paths. Guards against nil application context.
  • module/pipeline_step_auth_validate_test.go — 13 tests covering success, custom config, missing/malformed/invalid tokens, auth errors, HTTP response writing, nil app, and required field validation.
  • plugins/pipelinesteps/plugin.go — Register factory and manifest entry.
  • plugins/pipelinesteps/plugin_test.go — Update expected step count.

Config

Field Type Default Description
auth_module string (required) Service name of the AuthProvider module
token_source string (required) Dot-path to the token in pipeline context (e.g. steps.parse-request.headers.Authorization)
subject_field string auth_user_id Output key mapped from the sub claim

Usage

- name: auth
  type: step.auth_validate
  config:
    auth_module: m2m-auth
    token_source: steps.parse-request.headers.Authorization

On success, all claims returned by the AuthProvider are emitted as flat output keys (sub, scope, iss, etc.) plus the configured subject_field mapped from sub. On failure, pipeline stops with a 401 JSON response.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…ation

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
@intel352 intel352 marked this pull request as ready for review February 27, 2026 05:33
Copilot AI review requested due to automatic review settings February 27, 2026 05:33
Copilot AI changed the title [WIP] Add JWT/Bearer token validation step feat: add step.auth_validate pipeline step Feb 27, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new step.auth_validate pipeline step to validate HTTP Bearer/JWT-style tokens via an AuthProvider, and wires it into the pipeline-steps plugin so it can be used from YAML-configured pipelines.

Changes:

  • Introduces AuthValidateStep (step.auth_validate) with configurable auth_module, token_source, and subject_field.
  • Adds a dedicated unit test suite for the new step (11 tests).
  • Registers the new step type/factory in plugins/pipelinesteps/plugin.go and updates plugin loader expectations.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
module/pipeline_step_auth_validate.go Implements the new auth validation pipeline step and its 401 response behavior.
module/pipeline_step_auth_validate_test.go Adds unit tests covering success, configuration options, and unauthorized scenarios.
plugins/pipelinesteps/plugin.go Registers step.auth_validate in the plugin manifest and factory map.
plugins/pipelinesteps/plugin_test.go Updates step lists/count assertions to include the new step.

Comment thread module/pipeline_step_auth_validate.go Outdated
Comment thread module/pipeline_step_auth_validate.go
Comment thread module/pipeline_step_auth_validate.go
@intel352
Copy link
Copy Markdown
Contributor

@copilot apply changes based on the comments in this thread

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
@intel352 intel352 merged commit cdee55d into main Feb 27, 2026
14 checks passed
@intel352 intel352 deleted the copilot/add-auth-validation-step branch February 27, 2026 06:01
intel352 added a commit that referenced this pull request May 5, 2026
…ries (workflow#541)

buildAlignContext now populates ctx.secretKeys from
cfg.Secrets.Generate[i].Key and cfg.Secrets.Entries[i].Name in addition
to the existing module-form (secrets.generate / secrets.requires) path.
This unblocks core-dump PR #190 which uses top-level secrets.generate to
declare STAGING_PG_PASSWORD; before this fix wfctl infra align --strict
fired a false-positive R-A4 FAIL because the unresolved env var
${STAGING_PG_PASSWORD} couldn't be matched against a known secret key.

Adds two pinning tests:
- TestInfraAlign_RA4_TopLevelSecretsGenerate_DoesNotFire
- TestInfraAlign_RA4_TopLevelSecretsEntries_DoesNotFire

Existing module-form R-A4 tests remain green (no regression).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
intel352 added a commit that referenced this pull request May 5, 2026
…ries (workflow#541) (#550)

* fix(wfctl): R-A4 align rule consults top-level secrets.generate + entries (workflow#541)

buildAlignContext now populates ctx.secretKeys from
cfg.Secrets.Generate[i].Key and cfg.Secrets.Entries[i].Name in addition
to the existing module-form (secrets.generate / secrets.requires) path.
This unblocks core-dump PR #190 which uses top-level secrets.generate to
declare STAGING_PG_PASSWORD; before this fix wfctl infra align --strict
fired a false-positive R-A4 FAIL because the unresolved env var
${STAGING_PG_PASSWORD} couldn't be matched against a known secret key.

Adds two pinning tests:
- TestInfraAlign_RA4_TopLevelSecretsGenerate_DoesNotFire
- TestInfraAlign_RA4_TopLevelSecretsEntries_DoesNotFire

Existing module-form R-A4 tests remain green (no regression).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(config): processImports merges top-level Secrets (workflow#541)

processImports previously dropped imported WorkflowConfig.Secrets blocks,
so a `secrets:` declaration in shared.yaml never reached the loaded
parent config. R-A4 (and R-A9) consult cfg.Secrets via buildAlignContext;
without the merge, an env var resolved by an imported secrets.generate /
secrets.entries entry continued to false-positive as unresolved.

Merge semantics match the rest of processImports:
- DefaultStore / Provider / Config / Rotation: parent wins; imported
  value adopted only when parent's field is unset.
- Generate slice: append imported entries; dedupe by Key (parent first).
- Entries slice: append imported entries; dedupe by Name (parent first).

Tests:
- config: TestLoadFromFile_ImportSecretsMerge (precedence + dedupe);
          TestLoadFromFile_ImportSecretsOnlyInImport (cfg.Secrets nil
          before fix, populated after).
- wfctl: TestInfraAlign_RA4_TopLevelSecrets_FromImport_DoesNotFire
          (verified to FAIL without the merge — proves the bug + fix).

Addresses Copilot inline review on PR #550.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(config): processImports merges SecretStores + per-key Secrets.Config (workflow#541)

Two follow-on fixes from Copilot round-2 review on PR #550:

1. SecretStores merge: ResolveSecretStore / getProviderForStore look up
   store names against WorkflowConfig.SecretStores. processImports
   previously dropped that map entirely; an imported `defaultStore` or
   `entries[*].store` reference would later be treated as a raw provider
   name and fail with unknown-provider. Now merged with parent-wins
   per-key dedupe (matching Workflows / Triggers / Pipelines / Platform
   / Plugins.External semantics).

2. Secrets.Config per-key merge: previously the entire imported map was
   discarded as soon as the main file defined any key, breaking the
   shared-defaults + local-override pattern (e.g. import provides
   {repo, token_env, api_url}; main only overrides {token_env}). Now
   per-key merge: imported keys absent from main survive, main wins
   on key conflicts.

3. Comment-typo fix in config_import_test.go (missing space).

Tests:
- TestProcessImports_MergesSecretStoresFromImport
- TestProcessImports_SecretsConfigMergesPerKey_LocalOverride
Both verified to FAIL without their respective fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(config): processImports merges Environments + canonical provider IDs (workflow#541)

Round-3 review surfaced a third missing merge surface in processImports.
This commit closes it and audits the remaining WorkflowConfig fields so
the cascade ends here.

Field audit of WorkflowConfig vs processImports merge coverage:

  Already merged BEFORE this PR:
    Modules, Workflows, Triggers, Pipelines, Platform, Plugins.External,
    Sidecars

  Merged in earlier commits of this PR (#550):
    Secrets (Generate dedupe-by-Key, Entries dedupe-by-Name, Config
    per-key, scalars parent-wins), SecretStores (per-store dedupe-by-name)

  Merged in THIS commit:
    Environments (per-env dedupe-by-name, parent-wins) — closes the
    finding that ResolveSecretStore consults
    Environments[env].SecretsStoreOverride and dropping it routes
    secrets to the wrong backend.

  Intentionally NOT merged (out-of-scope follow-ups for #541):
    Requires, Infrastructure, Engine, CI, Infra, Services, Mesh,
    Networking, Security — these are all single-document config blocks
    where merge semantics need separate design + dedicated regression
    tests; will file follow-up issues.

Test fixture cleanup (Copilot round-3 nits): use canonical provider IDs
`aws-secrets-manager` and `gcp-secret-manager` (per docs/dsl-reference.md
and cmd/wfctl/wizard.go). Fixtures double as copy-paste examples; using
unsupported spellings in tests is a footgun.

Tests:
- TestProcessImports_MergesEnvironmentsFromImport — verified to FAIL
  without the new merge (`expected Environments[staging] from import`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
intel352 added a commit that referenced this pull request May 9, 2026
#586)

Records the closeout for Tasks 5+6 of the spaces-key-iac-resource plan
(docs/plans/2026-05-08-spaces-key-iac-resource.md, commit 316559f7).

The plan's PR2 was specified as a migration of core-dump/infra.yaml from
a two-entry SPACES_access_key/SPACES_secret_key provider_credential
schema to canonical single-entry. At impl time, verification against
origin/main HEAD 3bb46833 showed the file already had the canonical
shape — the migration had landed in PR #190 (TC1 cutover) or PR #194
(TC2 cutover) before the plan was authored.

Smoke-confirmed: `wfctl infra align --strict -c infra.yaml --env staging`
returns exit 0 with "No alignment issues found." — no R-A9 firing
because there's nothing to fire on.

Tasks 5+6 are marked completed as a no-op confirmation. PR1's R-A9
severity flip (workflow #583, merged) provides the ongoing regression
protection: any future reintroduction of the two-entry shape will hit
ERROR R-A9 at align-strict time, exit 1.

ADR also captures the planner-blindspot lesson (operator memory about
file shapes is unreliable; re-fetch origin/main before locking a plan
that mutates external repo files) for the post-merge retro.

Per team-lead's user-direction routing of the (a/b/c) options I had
surfaced.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants