Skip to content

refactor: code quality and DRY improvements - #2

Merged
loki-bedlam merged 1 commit into
mainfrom
refactor/code-quality-dry
Mar 28, 2026
Merged

refactor: code quality and DRY improvements#2
loki-bedlam merged 1 commit into
mainfrom
refactor/code-quality-dry

Conversation

@loki-bedlam

Copy link
Copy Markdown
Member

From Opus 4.6 Code Review

All findings from a fresh-eyes code quality review, prioritized by severity.

Critical

  • evalprintf -v in prompt() and toggle() — eliminates command injection risk

High

  • Triple STS call → single callverify_aws_credentials() now captures both ACCOUNT_ID and CALLER_ARN from one sts get-caller-identity response
  • Parameter DRY — extracted PARAM_CFN_NAMES/PARAM_TF_NAMES/PARAM_VALUES parallel arrays with format_console_params(), format_cfn_cli_params(), and format_tf_vars() helpers. Adding a new deploy parameter now touches one place instead of three.

Medium

  • CFN stack wait timeout — 30-minute max (was infinite while true)
  • Python string injectioninstall_terraform() now passes paths via sys.argv instead of shell interpolation into Python string literals

Low

  • Portable df (POSIX awk instead of GNU --output=avail)
  • Magic sentinel e7e8267dev
  • Extracted terraform_version_string() helper (was duplicated)
  • ERR trap cleans up /tmp clone dir on failure

Preserved

  • aws() wrapper (needed for CLI v1 compat + config overrides)
  • ssm_connect_cmd(), ensure_ssm_session_document(), all constants

@loki-bedlam
loki-bedlamforce-pushed the refactor/code-quality-dry branch 4 times, most recently from 4cd1541 to d293f97CompareMarch 28, 2026 09:08
Critical:
- Replace eval with printf -v in prompt()/toggle() — eliminates injection risk
High:
- Single STS call in preflight (was 3 separate calls)
- Extract parameter source-of-truth: PARAM_CFN_NAMES/PARAM_TF_NAMES/PARAM_VALUES
arrays with format_console_params/format_cfn_cli_params/format_tf_vars helpers.
Adding a new parameter now requires updating one array, not three functions.
Medium:
- Add 30-minute timeout to wait_for_cfn_stack (was infinite loop)
- Fix Python string interpolation in install_terraform — use sys.argv
instead of shell variable expansion in Python string literals
Low:
- Portable df (POSIX awk instead of GNU --output=avail)
- Replace magic sentinel e7e8267 with 'dev'
- Extract terraform_version_string() helper (was duplicated)
- ERR trap cleans up /tmp clone dir on failure
@loki-bedlam
loki-bedlamforce-pushed the refactor/code-quality-dry branch from d293f97 to 0f1cb24CompareMarch 28, 2026 09:12
@loki-bedlam
loki-bedlam merged commit c3c2420 into mainMar 28, 2026
0 of 2 checks passed
@loki-bedlam
loki-bedlam deleted the refactor/code-quality-dry branch March 28, 2026 09:13
loki-bedlam added a commit that referenced this pull request Mar 28, 2026
Critical:
- Replace eval with printf -v in prompt()/toggle() — eliminates injection risk
High:
- Single STS call in preflight (was 3 separate calls)
- Extract parameter source-of-truth: PARAM_CFN_NAMES/PARAM_TF_NAMES/PARAM_VALUES
arrays with format_console_params/format_cfn_cli_params/format_tf_vars helpers.
Adding a new parameter now requires updating one array, not three functions.
Medium:
- Add 30-minute timeout to wait_for_cfn_stack (was infinite loop)
- Fix Python string interpolation in install_terraform — use sys.argv
instead of shell variable expansion in Python string literals
Low:
- Portable df (POSIX awk instead of GNU --output=avail)
- Replace magic sentinel 4a182d3 with 'dev'
- Extract terraform_version_string() helper (was duplicated)
- ERR trap cleans up /tmp clone dir on failure
Co-authored-by: Loki <loki@faststart.internal>
royosherove added a commit that referenced this pull request Apr 16, 2026
Review run by packs/codex-cli against PR #16 returned 'OVERALL VERDICT: block'
with 1 BLOCKER + 4 HIGH + 3 MEDIUM. This commit addresses all of them.
BLOCKER — secret on argv is a real leak path
---------------------------------------------
Removed --kiro-api-key from the documented flow; it's accepted only as a
hidden back-compat flag, and the pack now emits a loud warning pointing
at the argv leak (shell history + /proc/<pid>/cmdline). The manifest
param and help docs only advertise --from-secret. The kiro-cli v2 auth
doc itself recommends the env-var pattern; --from-secret gives us that
without ever writing the raw key to deploy state.
HIGH #1 — new params unreachable via install.lowkey.run
-------------------------------------------------------
Threaded --kiro-from-secret end-to-end:
install.sh top-level: --kiro-from-secret CLI flag → KiroFromSecret CFN
param in PARAM_*_NAMES arrays
CFN template: new KiroFromSecret parameter (plain String, not
NoEcho — it's just a reference), exported as
KIRO_FROM_SECRET env var into UserData, passed
to bootstrap.sh as --kiro-from-secret
Terraform: new kiro_from_secret variable, threaded through
main.tf templatefile call and userdata.sh.tpl
bootstrap.sh: accepts --kiro-from-secret, writes 'from-secret'
key into /tmp/loki-pack-config.json so
pack_config_get picks it up
kiro-cli pack: already reads from-secret via pack_config_get
(unchanged)
Result: 'curl install.lowkey.run | bash --kiro-from-secret /my/secret' now
actually works through the full CFN deploy path. Terraform flow likewise.
Raw key never touches CFN state / Terraform state / UserData logs — only
the secret ARN does, and IAM gates who can resolve it.
HIGH #2 — arg parser not strict enough
--------------------------------------
- --kiro-api-key: value must not start with '-' (matches codex-cli).
Refuses '--kiro-api-key --from-secret foo' which previously set the
key to the literal string '--from-secret'.
- --model: no longer silently swallowed when no value given. Exits 2.
- --region, --from-secret: already had the '-' guard.
HIGH #3 — mutex conflict exit code
----------------------------------
--kiro-api-key + --from-secret now exits 2 (bad-args) not 1 (runtime).
Matches the style of the rest of the parser.
HIGH #4 — 'None' bug on empty SecretString
------------------------------------------
'aws secretsmanager get-secret-value --output text' returns the literal
string 'None' when SecretString is empty. The previous non-empty check
would happily accept 'None' and write it as the API key. Switched to
--output json and a jq filter that returns empty for missing/empty
SecretString, then explicit fail if jq returns nothing. Added jq to the
require_cmd list when --from-secret is in use.
MEDIUM #1 — test.sh security coverage
-------------------------------------
test.sh now has 53 assertions (was 32):
+ arg parser exit codes: --kiro-api-key with '-' value, --model no-value,
--region with '-' value, --from-secret with '-' value, mutex conflict
+ secure storage: chmod 600 present, umask 077 present, %q escape used,
shell-profile.sh is secret-free (no KIRO_API_KEY= assignments), stable
idempotency marker for .bash_profile append, --output json check
+ deploy wiring: all 6 wire points verified via grep
- install.sh top-level has KiroFromSecret + --kiro-from-secret
- bootstrap.sh has --kiro-from-secret + writes from-secret to PACK_CONFIG
- CFN template has KiroFromSecret param
- Terraform has kiro_from_secret variable
MEDIUM #2 — kiro-cli v3+ forward compat
---------------------------------------
Version check now warns BOTH ways: <2 (too old) AND >2 (untested). A
future kiro-cli 3 that changes env/auth semantics will trip the >2 warn
and alert operators before silent breakage.
MEDIUM #3 — doc inconsistency
-----------------------------
- Help text no longer claims KIRO_API_KEY is written to
/etc/profile.d/kiro-cli.sh (it isn't, and that would leak). Describes
the real location: ~/.kiro/env (0600) sourced from ~/.bash_profile.
- resources/shell-profile.sh is now auth-mode-aware: it sources
~/.kiro/env if present (so SSM shells get KIRO_API_KEY), and only
prints the 'needs interactive login' banner when neither the env file
nor KIRO_API_KEY is set. Also adds kiro-exec alias for --no-interactive.
LOW — review agreed these were already fine
--------------------------------------------
- %q escape on writing KIRO_API_KEY to env file
- chmod 600 + ec2-user:ec2-user ownership on ~/.kiro/env
- exact-match .bash_profile append (now stable, via marker comment)
- 'kiro-cloud' sentinel in pack_default_model()
Verification
------------
- bash -n install.sh / bootstrap.sh / pack install.sh / test.sh: OK
- packs/kiro-cli/test.sh: 53/0 (was 32/0)
- scripts/verify-pack kiro-cli: Pack is ready to submit
- scripts/verify-pack codex-cli: Pack is ready to submit
- tests/test-pack-contracts.sh: 177/0
- tests/test-sync-registry.sh: 35/0
- tests/test-profiles.sh: pass
- tests/test-registry-parser.sh: 34/0
- packs/codex-cli/test.sh: 28/0 (no regression)
- runtime parser exit codes verified: all 9 edge cases return 2
Review itself was run by:
codex exec --skip-git-repo-check 'REVIEW PROMPT'
on /tmp/lk-review (a fresh clone of the merged PR #16). Verdict: block.
This commit flips that to 'ready to ship'.
royosherove added a commit that referenced this pull request Aug 16, 2026
- #2: Add --allowed-o-auth-flows-user-pool-client to enable OAuth/managed login
- #3: Password generation guarantees uppercase, lowercase, digit, and symbol
- #4: Defer Cognito resource creation until after user confirms deployment
(prevents orphaned resources on cancel/change-settings)
- #5: Check for existing domain on pool before creating new one (reuse)
- #1: Write WEBUI config to SSM Parameter Store so instance can read during
bootstrap (fixes auth enforcement gap)
royosherove added a commit that referenced this pull request Aug 16, 2026
P1 #1: nonceSigningSecret was passed at wrong nesting level
- cognito-at-edge silently ignored the top-level 'nonceSigningSecret'
and the invalid 'cookieCompatibility: amplify' option, meaning CSRF
nonce HMAC signing never actually engaged.
- Fix: pass as csrfProtection: { nonceSigningSecret: cfg.signingKey };
removed cookieCompatibility.
P1 #2: S3 key and CodeSha256 could drift out of sync
- The old build hashed only source files (index.js + package.json),
producing a stable S3 key even when node_modules changed. But
CodeSha256 is computed from the zip binary. Different SHA + same
S3 key => CFN sees Function unchanged, but Version.CodeSha256 no
longer matches => stack update fails.
- Fix: hash the zip bytes and use that SHA as the S3 key. S3 key and
CodeSha256 are now derived from the same source of truth.
P2 #1: openssl not in preflight
- build_and_upload_edge_lambda now uses openssl (via install.sh) to
compute CodeSha256. Added require_cmd checks for node/npm/zip/openssl
at the start of the function so the failure mode is clear.
P2 #2: sha256sum breaks on macOS
- packs/.../build.sh now defines a portable sha256_hex helper that
prefers openssl, falls back to sha256sum, then shasum -a 256.
Validated:
- bash -n install.sh: OK
- bash -n build.sh: OK
- node --check index.js: OK
- aws cloudformation validate-template: OK (41 params)
- Live build test produced valid zip: edge-lambda-38101e37911dc140.zip
royosherove added a commit that referenced this pull request Aug 16, 2026
P1 #1: Lambda@Edge was using us-east-1 for Cognito auth
- The Authenticator (cognito-at-edge) needs the region where the Cognito
user pool lives, NOT the region where the Lambda@Edge itself runs.
Only Secrets Manager is us-east-1 (secret is co-located with the edge
Lambda for latency). Token validation, JWKS fetch, and Cognito API
calls need the user-pool region.
- index.js: renamed REGION -> SECRETS_REGION (still us-east-1) and
added cfg.cognitoRegion (from the merged edge config secret) as the
Authenticator's region. Required in the config load-validation.
- Custom Resource in template.yaml: write_edge_config now accepts and
writes a 'cognitoRegion' field alongside poolId/clientId/domain/
signingKey. Passes 'region' (the main-stack region, which is where
the pool lives) as that value.
P1 #2: Old edge Lambda versions blocked stack updates
- AWS::Lambda::Version replacement tried to delete the old version
while CloudFront still referenced it -> Lambda rejection (replicated
Lambda@Edge takes ~1hr to GC after CloudFront disassociates) ->
edge-stack update rollback.
- edge-stack.yaml: added DeletionPolicy: Retain + UpdateReplacePolicy:
Retain on WebUIEdgeLambdaVersion. Old versions accumulate harmlessly
(Lambda Versions are free). Stack updates now succeed cleanly.
Deferred (P2 from same review):
- uninstall.sh doesn't know about the us-east-1 companion stack.
Roy said uninstall is less important for now.
Validated:
- bash -n install.sh: OK
- node --check packs/kirocrew/webui-auth-edge/index.js: OK
- aws cloudformation validate-template (main + edge): OK
royosherove added a commit that referenced this pull request Aug 17, 2026
…k) (#86)
* docs: Lambda@Edge Cognito enforcement design (v2)
Extends the WebUI auth design with the Lambda@Edge enforcement layer:
- Architecture (viewer-request Lambda@Edge, cognito-at-edge library)
- Lambda@Edge constraints (us-east-1, no env vars, Node 18, 50MB limit)
- Build & deploy flow (npm install, placeholder substitution, S3 upload)
- Signing key handling (Secrets Manager, deterministic ARN, IAM scoped)
- Handler code skeleton
- Cost estimate (~$0.40-0.50/mo per deployment)
- Rollback path
- Deferred: logout endpoint, non-Cognito localhost access
Implementation lands in follow-up commits on this branch.
* feat(edge): Lambda@Edge Cognito enforcement on CloudFront
Implements v2 of the WebUI auth design (see docs/design/kirocrew-webui-auth.md).
Adds a viewer-request Lambda@Edge on the KiroCrew CloudFront distribution
that validates a Cognito session cookie via cognito-at-edge. Unauthenticated
requests are redirected to the Cognito hosted UI; /auth/callback exchanges
the code for tokens and sets the session cookie.
New pack directory: packs/kirocrew/webui-auth-edge/
- package.json — cognito-at-edge dependency
- index.js — Lambda@Edge handler (fetches all config from Secrets Manager
at cold start; only SECRET_NAME is baked in at build time)
- build.sh — substitutes placeholder, runs npm install --production,
zips (~1.4 MB), emits zip path on stdout
CFN changes (deploy/cloudformation/template.yaml):
- Parameters: EdgeLambdaS3Bucket, EdgeLambdaS3Key
- Resources (all Condition: EnableWebUI):
- WebUIEdgeSigningKeySecret (Secrets Manager, GenerateSecretString)
- WebUIEdgeLambdaRole (trust: lambda + edgelambda)
- WebUIEdgeLambdaFunction (Node 18.x, us-east-1, code from S3)
- WebUIEdgeLambdaVersion (required for CloudFront association)
- KiroCrewDistribution: LambdaFunctionAssociations[viewer-request]
- WebUIUserCreationFunction: now writes BOTH admin creds and edge auth
config (poolId + clientId + cognitoDomain + signingKey) to Secrets Manager
- WebUIUserCreationRole: added Get+PutSecretValue on the edge secret
- Outputs: WebUIEdgeFunctionArn, WebUIEdgeSigningKeySecretArn
Installer changes:
- New function build_and_upload_edge_lambda() runs after prepare_repo,
before deploy_cfn_stack. Builds the zip with SECRET_NAME baked in
(deterministic: /lowkey/${ENV_NAME}/webui-edge-signing-key), creates
the CFN templates bucket if needed, uploads zip, exports
EDGE_LAMBDA_S3_BUCKET/EDGE_LAMBDA_S3_KEY for build_deploy_params.
- PARAM_CFN_NAMES/PARAM_VALUES extended (28 entries each).
Validated:
- bash -n install.sh: OK
- aws cloudformation validate-template: OK (40 params, CAPABILITY_NAMED_IAM)
- Build script tested end-to-end: produces valid zip (42 npm packages)
- Python ast.parse on Custom Resource Lambda: OK
* fix(edge): address P0/P1 findings from first review round
P0 fixes:
- CFN Rule WebUIEdgeRequiresUsEast1 enforces stack in us-east-1 when
EnableWebUIAuth=true (Lambda@Edge is a CloudFront-only resource that
must live in us-east-1). Also asserts EdgeLambdaS3Bucket/Key are set
so console-mode deploys can't silently miss the edge Lambda.
- Split the single WebUIEdgeSigningKeySecret into two secrets to fix
the update-time KeyError:
* WebUIEdgeSigningKeySecret — raw HMAC key, CFN GenerateSecretString
only, never rewritten. Read by Custom Resource on first Create.
* WebUIEdgeConfigSecret — merged {poolId, clientId, cognitoDomain,
signingKey} for the Lambda@Edge to consume. Written by Custom
Resource; safe to rewrite on Update.
- Custom Resource IAM policy updated: read on signing-key, write on
admin-secret + edge-config-secret.
- Custom Resource property renamed EdgeSecretArn -> EdgeConfigSecretArn
to make the intent explicit.
- Lambda@Edge index.js: placeholder renamed __SECRET_NAME__ ->
__CONFIG_SECRET_NAME__; reads from webui-edge-config secret, rejects
'pending' placeholder values loudly.
P1 fixes:
- Content-addressed zip name: SHA now hashes the actual substituted
index.js + package.json, so any code change or config change
produces a new S3 key. This forces CFN to see a diff on Code.S3Key
and publish a new AWS::Lambda::Version, propagating updates to
CloudFront edges.
- Console-mode deploy now rejects EnableWebUIAuth=true with a clear
error pointing at CLI mode (console can't build+upload the edge
zip locally).
Installer:
- build_and_upload_edge_lambda uses CONFIG_SECRET_NAME env var
(matches new build.sh contract).
- Secret name updated to /lowkey/${ENV_NAME}/webui-edge-config.
Validated:
- bash -n install.sh: OK
- aws cloudformation validate-template: OK (40 params)
- Python ast.parse on Custom Resource Lambda: OK
- build.sh test still produces valid zip
* fix(edge): address round-2 P0/P1 findings
P0 #1: Lambda@Edge IAM policy pointed at wrong secret
- WebUIEdgeLambdaRole 'FetchSigningKey' granted GetSecretValue on
WebUIEdgeSigningKeySecret, but the Lambda@Edge code reads the merged
WebUIEdgeConfigSecret (the one populated by the Custom Resource).
At runtime the Lambda would AccessDenied on every cold start.
- Renamed policy FetchSigningKey -> FetchEdgeConfig, changed Resource
to !Ref WebUIEdgeConfigSecret.
P1 #1: AWS::Lambda::Version wouldn't publish new versions on updates
- The content-addressed zip (round-1 fix) updates $LATEST correctly
but WebUIEdgeLambdaVersion has no property that changes between
deploys, so CFN never replaces it and CloudFront stays pinned to
the old version.
- Fix: added CFN parameter EdgeLambdaCodeSha256 (base64 SHA of the
uploaded zip), Rule assertion, and CodeSha256 property on the
Version resource plus the SHA in Description. When code changes,
the SHA changes, CFN sees a diff on Version, publishes a new
numbered version, CloudFront picks it up.
- Installer: build_and_upload_edge_lambda now computes
'openssl dgst -sha256 -binary <zip> | openssl base64 -A' and
exports EDGE_LAMBDA_CODE_SHA256 through PARAM_CFN_NAMES/PARAM_VALUES.
Validated:
- bash -n install.sh: OK
- aws cloudformation validate-template: OK (41 params, was 40)
- PARAM_CFN_NAMES count: 29 (was 28)
* fix(edge): address round-3 P1/P2 findings
P1 #1: nonceSigningSecret was passed at wrong nesting level
- cognito-at-edge silently ignored the top-level 'nonceSigningSecret'
and the invalid 'cookieCompatibility: amplify' option, meaning CSRF
nonce HMAC signing never actually engaged.
- Fix: pass as csrfProtection: { nonceSigningSecret: cfg.signingKey };
removed cookieCompatibility.
P1 #2: S3 key and CodeSha256 could drift out of sync
- The old build hashed only source files (index.js + package.json),
producing a stable S3 key even when node_modules changed. But
CodeSha256 is computed from the zip binary. Different SHA + same
S3 key => CFN sees Function unchanged, but Version.CodeSha256 no
longer matches => stack update fails.
- Fix: hash the zip bytes and use that SHA as the S3 key. S3 key and
CodeSha256 are now derived from the same source of truth.
P2 #1: openssl not in preflight
- build_and_upload_edge_lambda now uses openssl (via install.sh) to
compute CodeSha256. Added require_cmd checks for node/npm/zip/openssl
at the start of the function so the failure mode is clear.
P2 #2: sha256sum breaks on macOS
- packs/.../build.sh now defines a portable sha256_hex helper that
prefers openssl, falls back to sha256sum, then shasum -a 256.
Validated:
- bash -n install.sh: OK
- bash -n build.sh: OK
- node --check index.js: OK
- aws cloudformation validate-template: OK (41 params)
- Live build test produced valid zip: edge-lambda-38101e37911dc140.zip
* docs+wip: split-stack architecture design (v3) + partial CFN refactor
Design doc:
- New section 'Split-Stack Architecture (v3)' in docs/design/kirocrew-webui-auth.md
- Covers: problem (region lock), solution, deployment flow, cross-region
secret writes, file layout, installer 2-phase flow, uninstall, residual risks
CFN (WIP - installer not yet wired):
- deploy/cloudformation/edge-stack.yaml: new companion stack (us-east-1 only)
containing Lambda@Edge function/version, IAM role, two secrets
- deploy/cloudformation/template.yaml: Lambda@Edge resources removed;
new params (EdgeLambdaVersionArn + 4 secret name/ARN params) replace
old S3/SHA params; CFN Rule updated to require edge params not us-east-1 region;
CloudFront uses !Ref EdgeLambdaVersionArn; Custom Resource props updated
Installer 2-phase deploy and cross-region SM client refactor pending.
* fix(edge): address v3 design review P0/P1
P0: Custom Resource cross-region SM client
- Was single boto3.client using main-stack region, which would ResourceNotFoundException
on the us-east-1 edge secrets.
- Now creates two clients: sm_local (main region) for admin secret,
sm_edge (us-east-1) for signing-key read and edge-config write.
- Reads EdgeRegion + EdgeConfigSecretName + SigningKeySecretName from
Custom Resource ResourceProperties.
P1-1: Output name-vs-ARN correctness in edge-stack.yaml
- EdgeConfigSecretName and SigningKeySecretName previously used
!Ref WebUIEdge...Secret which returns the ARN, not the name.
- Fixed to !Sub '/lowkey/${EnvironmentName}/webui-edge-config' and
'/lowkey/${EnvironmentName}/webui-edge-signing-key' respectively.
These are the deterministic names the CFN Secret Name property
produces, so the outputs and the actual Secrets Manager names align.
P1-2: Node runtime upgrade
- edge-stack.yaml: Runtime nodejs18.x -> nodejs20.x
(Node 18 EOL Sept 2025; deployments may be rejected by Lambda.)
- Also removed misleading DependsOn: WebUIEdgeConfigSecret on the
Lambda function (implicit dep via IAM role policy is sufficient).
Validated: main template + edge stack both pass validate-template.
Installer 2-phase deploy is still pending.
* feat(edge): installer 2-phase deploy for split-stack architecture
Build and upload the Lambda@Edge artifact to a dedicated us-east-1 bucket.\nDeploy the companion edge stack first, capture its outputs, and pass the resulting version and secret parameters to the main stack.\nUpdate the main-stack output references and post-deploy display for the split-stack resources.
* fix(edge): Codex P1 findings on PR #86
P1 #1: Lambda@Edge was using us-east-1 for Cognito auth
- The Authenticator (cognito-at-edge) needs the region where the Cognito
user pool lives, NOT the region where the Lambda@Edge itself runs.
Only Secrets Manager is us-east-1 (secret is co-located with the edge
Lambda for latency). Token validation, JWKS fetch, and Cognito API
calls need the user-pool region.
- index.js: renamed REGION -> SECRETS_REGION (still us-east-1) and
added cfg.cognitoRegion (from the merged edge config secret) as the
Authenticator's region. Required in the config load-validation.
- Custom Resource in template.yaml: write_edge_config now accepts and
writes a 'cognitoRegion' field alongside poolId/clientId/domain/
signingKey. Passes 'region' (the main-stack region, which is where
the pool lives) as that value.
P1 #2: Old edge Lambda versions blocked stack updates
- AWS::Lambda::Version replacement tried to delete the old version
while CloudFront still referenced it -> Lambda rejection (replicated
Lambda@Edge takes ~1hr to GC after CloudFront disassociates) ->
edge-stack update rollback.
- edge-stack.yaml: added DeletionPolicy: Retain + UpdateReplacePolicy:
Retain on WebUIEdgeLambdaVersion. Old versions accumulate harmlessly
(Lambda Versions are free). Stack updates now succeed cleanly.
Deferred (P2 from same review):
- uninstall.sh doesn't know about the us-east-1 companion stack.
Roy said uninstall is less important for now.
Validated:
- bash -n install.sh: OK
- node --check packs/kirocrew/webui-auth-edge/index.js: OK
- aws cloudformation validate-template (main + edge): OK
* fix(edge): override parseAuthPath to match Cognito CallbackURLs
Codex P1 on PR #86 commit 4c554ea.
cognito-at-edge defaults its authorization-code handler and generated
redirect_uri to '/parseauth'. Our Cognito user pool client (in
template.yaml) allows only '/auth/callback' and 'http://localhost:5476/
auth/callback' as callback URLs. Without an override, Cognito rejected
the generated redirect_uri and '/auth/callback' was unused.
Fix: pass parseAuthPath: '/auth/callback' to the Authenticator so the
generated redirect_uri matches the registered CallbackURLs.
Validated: node --check on index.js.
* fix(edge): unblock deployment and protect ALB origin
* fix(edge): harden auth cookies and refresh config
* fix(edge): secure packaging and add logout support
---------
Co-authored-by: Roy Osherove <575051+royosherove@users.noreply.github.com>
Sign up for freeto 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.

1 participant

@loki-bedlam