UI/cli ux update#2
Merged
Merged
Conversation
Add multi-module codebase with CI pipeline
Replace the legacy survey-based interactive prompts with a Bubble Tea terminal UI wizard for `forge init`. Add --theme flag for TUI color theme selection. Add GitHub Actions job to auto-update the Homebrew formula on release.
initializ-mk
force-pushed
the
ui/cli-ux-update
branch
from
46f1c24 to
9d68ad8
Compare
Remove unused legacy prompt helpers (init_prompt.go), delete empty branch and unused slugRegexp in name_step, remove unused formatDomains in egress_step, and run gofmt across all TUI source files.
naveen-kurra
pushed a commit
to naveen-kurra/forge
that referenced
this pull request
May 23, 2026
…itializ#2) Before: newDiscovery trimmed the configured Issuer, but the discovery-doc check compared the trimmed value against the IdP's RAW emitted form, and jwt.WithIssuer received the user's RAW configured form. Result: any IdP that emits iss with a trailing slash (Auth0) was spuriously rejected against a config without slash, and vice versa. Reviewer flagged: provider.go vs discovery.go. Fix: - New Config.normalize() trims trailing slash from Issuer; called once in New() so every downstream comparison uses the same canonical form. - Discovery comparison now trims meta.Issuer before checking against the already-trimmed d.issuer. - JWT iss validation: replaced jwt.WithIssuer (strict equality, no trim option) with a manual checkIssuer that trims both the token's iss claim and the configured Issuer before comparing. Security: trimming a single trailing slash cannot collapse distinct issuers — "https://attacker.example" and "https://legit.example" remain distinct after trim. The trim is normalization, not weakening. TestIssuer_GenuineMismatchStillRejected guards this invariant. Backward compat: existing configs continue to work; new configs with trailing slashes now also work. JWT iss claim with trailing slash now also works. Tests: TestIssuer_ConfigWithoutSlash_TokenWithSlash (Auth0-shaped iss + Okta-shaped config) TestIssuer_ConfigWithSlash_TokenWithoutSlash (reverse) TestIssuer_BothWithoutSlash_StillWorks (regression — common case) TestIssuer_BothWithSlash_StillWorks (regression — Auth0 paired) TestIssuer_GenuineMismatchStillRejected (security guard) TestIssuer_TokenMissingIssClaim (no iss → ErrTokenRejected) TestDiscovery_TrailingSlashTolerated (discovery doc has slash, config doesn't) Verification: go test -race ./forge-core/auth/providers/oidc/ — all green full sweep — 47/47 packages pass golangci-lint v2.10.1 — 0 issues
16 tasks
9 tasks
naveen-kurra
pushed a commit
to naveen-kurra/forge
that referenced
this pull request
May 24, 2026
Batch-clearing the "don't block merge" follow-ups from review of PR initializ#79. initializ#1 gcp_iap classifyJWTErr — use jwt v5 sentinels via errors.Is rather than substring matching (library wording shifts across patches; sentinels are public API). Special-case ErrTokenSignatureInvalid to split alg-confusion (→ ErrInvalidToken) from real bad-signature (→ ErrTokenRejected) because golang-jwt wraps both under that one sentinel. Three internal keyFunc message-matches retained — those are strings WE control, not the library's. initializ#2 gcp_iap JWKS merge-on-success — switched j.keys = newKeys to a per-kid merge. A partial-but-valid JWKS response (e.g. one kid accidentally omitted by GCP during rotation) no longer drops kids the stale-grace contract assumes we still have. Worst case is keeping a retired kid in cache; verification still fails naturally for any token signed with the retired private key. initializ#3 azure_ad GraphCache defensive copies — Get returns append([]string(nil), e.groups...) and Put stores a copy of its input. Caller mutating Identity.Groups (the auth.Identity layer treats it as freely mutable) can't poison the cache. initializ#4 forge-cli needsYAMLQuoting numeric edge cases — quote anything that resembles a YAML number (hex 0x, octal 0o, binary 0b, leading-zero "010", scientific 1e10, decimal float 3.14, signed ±N, .inf / .nan in either case). Auth-setting values rarely hit these shapes but the docstring promised "false negatives are bugs" and the Web UI POST path can supply arbitrary strings. Added looksNumeric() helper with separate allHexDigits / allOctalDigits / allBinaryDigits gates. initializ#5 aws_sigv4 identity_cache_test — replaced string(rune(i)) with strconv.Itoa(i). Surrogate code points (0xD800..0xDFFF) all map to U+FFFD, so the eviction-threshold test was silently building ~10 distinct keys instead of 10_001 and the sweep never ran. initializ#6 http.NewRequestWithContext error handling — fixed the two `req, _ := ...` antipatterns in gcp_iap/iap_jwks.go and azure_ad/graph_client.go. Hardcoded URLs make the failure currently unreachable, but errcheck-clean is the discipline. initializ#7 gcp_iap HS256-with-EC-public-key alg-confusion test — pinned the most dangerous attack shape: attacker fetches the verifier's public key from JWKS (open by design), uses raw X/Y bytes as the HMAC "secret", signs an HS256 token. A non-whitelisting verifier would HMAC-verify it. Our keyFunc rejects on alg != "ES256" BEFORE key lookup; this test confirms. Tests added: TestGraphCache_GetReturnsDefensiveCopy, TestGraphCache_PutStoresDefensiveCopy, TestProvider_HS256WithECPublicKeyAsSecret_Rejected. Existing TestProvider_RS256Token_Rejected still passes (alg-confusion still classified as ErrInvalidToken under the new sentinel-based path). 42 packages green, lint + gofmt clean.
initializ-mk
pushed a commit
that referenced
this pull request
May 24, 2026
Batch-clearing the "don't block merge" follow-ups from review of PR #79. #1 gcp_iap classifyJWTErr — use jwt v5 sentinels via errors.Is rather than substring matching (library wording shifts across patches; sentinels are public API). Special-case ErrTokenSignatureInvalid to split alg-confusion (→ ErrInvalidToken) from real bad-signature (→ ErrTokenRejected) because golang-jwt wraps both under that one sentinel. Three internal keyFunc message-matches retained — those are strings WE control, not the library's. #2 gcp_iap JWKS merge-on-success — switched j.keys = newKeys to a per-kid merge. A partial-but-valid JWKS response (e.g. one kid accidentally omitted by GCP during rotation) no longer drops kids the stale-grace contract assumes we still have. Worst case is keeping a retired kid in cache; verification still fails naturally for any token signed with the retired private key. #3 azure_ad GraphCache defensive copies — Get returns append([]string(nil), e.groups...) and Put stores a copy of its input. Caller mutating Identity.Groups (the auth.Identity layer treats it as freely mutable) can't poison the cache. #4 forge-cli needsYAMLQuoting numeric edge cases — quote anything that resembles a YAML number (hex 0x, octal 0o, binary 0b, leading-zero "010", scientific 1e10, decimal float 3.14, signed ±N, .inf / .nan in either case). Auth-setting values rarely hit these shapes but the docstring promised "false negatives are bugs" and the Web UI POST path can supply arbitrary strings. Added looksNumeric() helper with separate allHexDigits / allOctalDigits / allBinaryDigits gates. #5 aws_sigv4 identity_cache_test — replaced string(rune(i)) with strconv.Itoa(i). Surrogate code points (0xD800..0xDFFF) all map to U+FFFD, so the eviction-threshold test was silently building ~10 distinct keys instead of 10_001 and the sweep never ran. #6 http.NewRequestWithContext error handling — fixed the two `req, _ := ...` antipatterns in gcp_iap/iap_jwks.go and azure_ad/graph_client.go. Hardcoded URLs make the failure currently unreachable, but errcheck-clean is the discipline. #7 gcp_iap HS256-with-EC-public-key alg-confusion test — pinned the most dangerous attack shape: attacker fetches the verifier's public key from JWKS (open by design), uses raw X/Y bytes as the HMAC "secret", signs an HS256 token. A non-whitelisting verifier would HMAC-verify it. Our keyFunc rejects on alg != "ES256" BEFORE key lookup; this test confirms. Tests added: TestGraphCache_GetReturnsDefensiveCopy, TestGraphCache_PutStoresDefensiveCopy, TestProvider_HS256WithECPublicKeyAsSecret_Rejected. Existing TestProvider_RS256Token_Rejected still passes (alg-confusion still classified as ErrInvalidToken under the new sentinel-based path). 42 packages green, lint + gofmt clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the legacy survey-based interactive prompts with a Bubble Tea terminal UI wizard for
forge init. Add --theme flag for TUI color theme selection. Add GitHub Actions job to auto-update the Homebrew formula on release.