Skip to content

Remove stub secondary_role from awsenc config#30

Merged
jgowdy-godaddy merged 1 commit intomainfrom
chore/remove-secondary-role-stub
Apr 17, 2026
Merged

Remove stub secondary_role from awsenc config#30
jgowdy-godaddy merged 1 commit intomainfrom
chore/remove-secondary-role-stub

Conversation

@jgowdy-godaddy
Copy link
Copy Markdown
Contributor

Summary

`secondary_role` was a config field that the parser accepted but every code path immediately errored with "chained role assumption is not supported yet":

  • `awsenc-cli/src/auth.rs::ensure_supported_secondary_role`
  • `awsenc-cli/src/serve.rs` try-transparent-reauth path

It's been shipped in that half-wired state since the aws-okta-processor migration landed, with no plan to implement actual chaining. The threat-model review flagged it as a footgun: a user who copies `secondary_role = { role_arn = "…" }` from a legacy config hits the error at `awsenc auth` time after typing their Okta password.

Rip out the dead code:

  • `ProfileConfig.secondary_role` field and `SecondaryRoleConfig` struct removed
  • `ResolvedConfig.secondary_role` field removed
  • `ensure_supported_secondary_role()` + its unit test removed
  • Runtime rejection blocks in `serve.rs` and `auth.rs` removed
  • Test fixtures (`cache_disk_tests.rs`, config tests) updated

Not removed

  • The migration tool's aws-okta-processor parser still detects and warns on `--secondary-role` in the source config and skips such profiles. That's a property of the other tool's config we read during migration, not awsenc's own schema.
  • `serde` is configured without `deny_unknown_fields`, so existing awsenc TOML with leftover `[secondary_role]` sections keeps loading cleanly (unknown field silently ignored). Added `resolve_config_ignores_legacy_secondary_role_field_in_toml` test to lock that behavior down.

`DESIGN.md` Phase 5 roadmap line for chained AssumeRole replaced with an explicit out-of-scope note pointing operators at SDK-side role chaining.

Test plan

  • `cargo test --workspace` — 272 tests pass across all 7 test binaries.
  • `cargo clippy --workspace --all-targets -- -D warnings` clean.
  • `cargo fmt --all -- --check` clean.
  • CI green on macOS / Linux / Windows.

secondary_role was a config field that the parser accepted but every
code path (auth.rs, serve.rs) immediately errored out of with
'chained role assumption is not supported yet'. It's been shipped in
that state since the aws-okta-processor migration path landed, with
no plan to implement chaining.

Rip it out:

- ProfileConfig.secondary_role field removed
- SecondaryRoleConfig struct removed
- ResolvedConfig.secondary_role field removed
- ensure_supported_secondary_role() helper + its unit test removed
- Runtime rejection in serve.rs + auth.rs removed
- cache_disk_tests.rs updated

Left alone deliberately:
- The migration tool's detection of aws-okta-processor's
  --secondary-role flag still warns and skips those profiles.
  That's a property of the OTHER tool's config we read during
  migration, not awsenc's own schema.
- serde is configured without deny_unknown_fields, so existing
  awsenc TOML with leftover [secondary_role] sections keeps loading
  cleanly; added a test covering that legacy config case.

DESIGN.md Phase 5 roadmap line for chained AssumeRole removed and
replaced with a brief 'out of scope' note pointing users at SDK-side
role chaining.
@jgowdy-godaddy jgowdy-godaddy merged commit e6adcb9 into main Apr 17, 2026
3 checks passed
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.

2 participants