SweetOps #pr-reviews for October, 2024

2024-10-01

2024-10-02

2024-10-04

RB

11:13:52 PM

May I get a review on this PR https://github.com/cloudposse/github-action-pre-commit/pull/26

#26 Bump the npm_and_yarn group with 2 updates

Bumps the npm_and_yarn group with 2 updates: webpack and undici.

Updates webpack from 5.90.1 to 5.94.0

Release notes

Sourced from webpack’s releases.

v5.94.0
Bug Fixes
• Added runtime condition for harmony reexport checked • Handle properly data/http/https protocols in source maps • Make bigint optimistic when browserslist not found • Move @types/eslint-scope to dev deps • Related in asset stats is now always an array when no related found • Handle ASI for export declarations • Mangle destruction incorrect with export named default properly • Fixed unexpected asi generation with sequence expression • Fixed a lot of types
New Features
• Added new external type “module-import” • Support webpackIgnore for new URL() construction • [CSS] @import pathinfo support
Security
• Fixed DOM clobbering in auto public path
v5.93.0
Bug Fixes
• Generate correct relative path to runtime chunks • Makes DefinePlugin quieter under default log level • Fixed mangle destructuring default in namespace import • Fixed consumption of eager shared modules for module federation • Strip slash for pretty regexp • Calculate correct contenthash for CSS generator options
New Features
• Added the binary generator option for asset modules to explicitly keep source maps produced by loaders • Added the modern-module library value for tree shakable output • Added the overrideStrict option to override strict or non-strict mode for javascript modules
v5.92.1
Bug Fixes
• Doesn’t crash with an error when the css experiment is enabled and contenthash is used
v5.92.0
Bug Fixes
• Correct tidle range’s comutation for module federation • Consider runtime for pure expression dependency update hash • Return value in the subtractRuntime function for runtime logic

… (truncated)

Commits • eabf85d chore(release): 5.94.0 • 955e057 security: fix DOM clobbering in auto public path • 9822387 test: fix • cbb86ed test: fix • 5ac3d7f fix: unexpected asi generation with sequence expression • 2411661 security: fix DOM clobbering in auto public path • b8c03d4 fix: unexpected asi generation with sequence expression • f46a03c revert: do not use heuristic fallback for “module-import” • 60f1898 fix: do not use heuristic fallback for “module-import” • 66306aa Revert “fix: module-import get fallback from externalsPresets” • Additional commits viewable in compare view

Updates undici from 5.28.3 to 5.28.4

Release notes

Sourced from undici’s releases.

v5.28.4
:warning: Security Release :warning:

• Fixes <https://github.com/nodejs/undici/security/advisories/GHSA-m4v8-wqvr-p9f7 “GHSA-m4v8-wqvr-p9f7” GHSA-m4v8-wqvr-p9f7> CVE-2024-30260
• Fixes <https://github.com/nodejs/undici/security/advisories/GHSA-9qxr-qj54-h672 “GHSA-9qxr-qj54-h672” GHSA-9qxr-qj54-h672> CVE-2024-30261
Full Changelog: nodejs/[email protected]…v5.28.4

Commits • fb98306 Bumped v5.28.4 • 2b39440 Merge pull request from https://github.com/advisories/GHSA-9qxr-qj54-h672 "GHSA-9qxr-qj54-h672"|GHSA-9qxr-qj54-h672 • 64e3402 Merge pull request from https://github.com/advisories/GHSA-m4v8-wqvr-p9f7 "GHSA-m4v8-wqvr-p9f7"|GHSA-m4v8-wqvr-p9f7 • 723c4e7 Revert “build(deps-dev): bump formdata-node from 4.4.1 to 6.0.3 (#2389)” • 0e9d54b skip failing test due to Node.js changes • See full diff in compare view

Dependabot will resolve any conflicts with this PR as long as you don’t alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

• @dependabot rebase will rebase this PR • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it • @dependabot merge will merge this PR after your CI passes on it • @dependabot squash and merge will squash and merge this PR after your CI passes on it • @dependabot cancel merge will cancel a previously requested merge and block automerging • @dependabot reopen will reopen this PR if it is closed • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency • @dependabot ignore <dependency name> major version will close this group update PR and stop Dependabot creating any more for the specific dependency’s major version (unless you unignore this specific dependency’s major version or upgrade to it yourself) • @dependabot ignore <dependency name> minor version will close this group update PR and stop Dependabot creating any more for the specific dependency’s minor version (unless you unignore this specific dependency’s minor version or upgrade to it yourself) • @dependabot ignore <dependency name> will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) • @dependabot unignore <dependency name> will remove all of the ignore conditions of the specified dependency • @dependabot unignore <dependency name> <ignore condition> will remove the ignore condition of the specified dependency and ignore conditions
You can disable automated security fix PRs for this repo from the Security Alerts page.

Gabriela Campana (Cloud Posse)

05:11:16 PM

@Igor Rodionov @Jeremy G (Cloud Posse)

#26 Bump the npm_and_yarn group with 2 updates

Bumps the npm_and_yarn group with 2 updates: webpack and undici.

Updates webpack from 5.90.1 to 5.94.0

Release notes

Sourced from webpack’s releases.

v5.94.0
Bug Fixes
• Added runtime condition for harmony reexport checked • Handle properly data/http/https protocols in source maps • Make bigint optimistic when browserslist not found • Move @types/eslint-scope to dev deps • Related in asset stats is now always an array when no related found • Handle ASI for export declarations • Mangle destruction incorrect with export named default properly • Fixed unexpected asi generation with sequence expression • Fixed a lot of types
New Features
• Added new external type “module-import” • Support webpackIgnore for new URL() construction • [CSS] @import pathinfo support
Security
• Fixed DOM clobbering in auto public path
v5.93.0
Bug Fixes
• Generate correct relative path to runtime chunks • Makes DefinePlugin quieter under default log level • Fixed mangle destructuring default in namespace import • Fixed consumption of eager shared modules for module federation • Strip slash for pretty regexp • Calculate correct contenthash for CSS generator options
New Features
• Added the binary generator option for asset modules to explicitly keep source maps produced by loaders • Added the modern-module library value for tree shakable output • Added the overrideStrict option to override strict or non-strict mode for javascript modules
v5.92.1
Bug Fixes
• Doesn’t crash with an error when the css experiment is enabled and contenthash is used
v5.92.0
Bug Fixes
• Correct tidle range’s comutation for module federation • Consider runtime for pure expression dependency update hash • Return value in the subtractRuntime function for runtime logic

… (truncated)

Commits • eabf85d chore(release): 5.94.0 • 955e057 security: fix DOM clobbering in auto public path • 9822387 test: fix • cbb86ed test: fix • 5ac3d7f fix: unexpected asi generation with sequence expression • 2411661 security: fix DOM clobbering in auto public path • b8c03d4 fix: unexpected asi generation with sequence expression • f46a03c revert: do not use heuristic fallback for “module-import” • 60f1898 fix: do not use heuristic fallback for “module-import” • 66306aa Revert “fix: module-import get fallback from externalsPresets” • Additional commits viewable in compare view

Updates undici from 5.28.3 to 5.28.4

Release notes

Sourced from undici’s releases.

v5.28.4
:warning: Security Release :warning:

• Fixes <https://github.com/nodejs/undici/security/advisories/GHSA-m4v8-wqvr-p9f7 “GHSA-m4v8-wqvr-p9f7” GHSA-m4v8-wqvr-p9f7> CVE-2024-30260
• Fixes <https://github.com/nodejs/undici/security/advisories/GHSA-9qxr-qj54-h672 “GHSA-9qxr-qj54-h672” GHSA-9qxr-qj54-h672> CVE-2024-30261
Full Changelog: nodejs/[email protected]…v5.28.4

Commits • fb98306 Bumped v5.28.4 • 2b39440 Merge pull request from https://github.com/advisories/GHSA-9qxr-qj54-h672 "GHSA-9qxr-qj54-h672"|GHSA-9qxr-qj54-h672 • 64e3402 Merge pull request from https://github.com/advisories/GHSA-m4v8-wqvr-p9f7 "GHSA-m4v8-wqvr-p9f7"|GHSA-m4v8-wqvr-p9f7 • 723c4e7 Revert “build(deps-dev): bump formdata-node from 4.4.1 to 6.0.3 (#2389)” • 0e9d54b skip failing test due to Node.js changes • See full diff in compare view

Dependabot will resolve any conflicts with this PR as long as you don’t alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

• @dependabot rebase will rebase this PR • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it • @dependabot merge will merge this PR after your CI passes on it • @dependabot squash and merge will squash and merge this PR after your CI passes on it • @dependabot cancel merge will cancel a previously requested merge and block automerging • @dependabot reopen will reopen this PR if it is closed • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency • @dependabot ignore <dependency name> major version will close this group update PR and stop Dependabot creating any more for the specific dependency’s major version (unless you unignore this specific dependency’s major version or upgrade to it yourself) • @dependabot ignore <dependency name> minor version will close this group update PR and stop Dependabot creating any more for the specific dependency’s minor version (unless you unignore this specific dependency’s minor version or upgrade to it yourself) • @dependabot ignore <dependency name> will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) • @dependabot unignore <dependency name> will remove all of the ignore conditions of the specified dependency • @dependabot unignore <dependency name> <ignore condition> will remove the ignore condition of the specified dependency and ignore conditions
You can disable automated security fix PRs for this repo from the Security Alerts page.

2024-10-07

Michael

06:49:21 PM

If it’s not too late to add this contribution to the AWS components, I can always hold of until after the migration (https://github.com/cloudposse/terraform-aws-components/pull/1143)

#1143 feat: add additional github repository options for argocd

what

• Add additional granular controls for ArgoCD repositories by supporting commit signing requirements and branch protection rules

why

• Add more flexibility and security into the existing Argo repo

testing

• This contribution is actively deployed within our downstream component library

Gabriela Campana (Cloud Posse)

05:11:53 PM

@Igor Rodionov @Ben Smith (Cloud Posse)

#1143 feat: add additional github repository options for argocd

what

• Add additional granular controls for ArgoCD repositories by supporting commit signing requirements and branch protection rules

why

• Add more flexibility and security into the existing Argo repo

testing

• This contribution is actively deployed within our downstream component library

Erik Osterman (Cloud Posse)

01:09:35 AM

I don’t believe it’s too late

1

Michael

01:10:08 AM

Merged thanks to Igor! Appreciate it

2024-10-08

david

09:13:41 PM

May I get a review on https://github.com/cloudposse/terraform-aws-components/pull/1149

#1149 feat: Add cross_origin_auth variable to auth0_client

what

• Adds the cross_origin_auth variable to the auth0_client resource

why

• Variable to allow cross-origin auth requests which is useful in CORS-heavy setups

references

• https://registry.terraform.io/providers/auth0/auth0/latest/docs/resources/client#cross_origin_auth

1

2024-10-09

Ray Finch

03:20:37 PM

Hey all, can I get a review of https://github.com/cloudposse/terraform-aws-dms/pull/36?

#36 Add support for map_boolean_as_boolean

what

Implemented postgres_settings for map_boolean_as_boolean as well as map_jsonb_as_clob and map_long_varchar_as.

why

map_boolean_as_boolean defaults to false and this is not what we want for our DMS.

references

closes #35

Gabriela Campana (Cloud Posse)

09:25:17 PM

@Andriy Knysh (Cloud Posse)

#36 Add support for map_boolean_as_boolean

what

Implemented postgres_settings for map_boolean_as_boolean as well as map_jsonb_as_clob and map_long_varchar_as.

why

map_boolean_as_boolean defaults to false and this is not what we want for our DMS.

references

closes #35

1

2024-10-10

2024-10-11

irl

12:12:37 PM

It’s been two weeks and I’m still waiting for a review on https://github.com/cloudposse/build-harness/pull/388. How long do reviews typically take?

#388 Allows overriding README_TEMPLATE_REPO_URL and README_ALLOWLIST_ORGS.

what

• Allows overriding README_TEMPLATE_REPO_URL and README_ALLOWLIST_ORGS.

why

• See #387 • tl;dr: unable to use this for non-CloudPosse organisations, and unable to use with GitLab

references

• Closes #387

Erik Osterman (Cloud Posse)

01:17:13 PM

I believe we made a conscious decision to not allow other organizations due to the potential supply chain attack vectors. We use this in all of our repos and the risk is too large.

#388 Allows overriding README_TEMPLATE_REPO_URL and README_ALLOWLIST_ORGS.

what

• Allows overriding README_TEMPLATE_REPO_URL and README_ALLOWLIST_ORGS.

why

• See #387 • tl;dr: unable to use this for non-CloudPosse organisations, and unable to use with GitLab

references

• Closes #387

Erik Osterman (Cloud Posse)

01:17:22 PM

Cc @Jeremy G (Cloud Posse)

irl

01:32:04 PM

@Erik Osterman (Cloud Posse) Does that mean the concept of build-harness-extensions is dead then?

irl

01:33:53 PM

i.e. you don’t expect other organisations to reuse build-harness

irl

01:37:20 PM

If so, this part of the readme needs an update because you will now always need to fork build-harness if you want readme functionality, and why wouldn’t you want a readme. https://github.com/cloudposse/build-harness?tab=readme-ov-file#extending-build-harness-with-targets-from-another-repo

Erik Osterman (Cloud Posse)

02:29:49 PM

Let me discuss this with @Jeremy G (Cloud Posse) and see what we can do

1

Jeremy G (Cloud Posse)

04:29:30 AM

I didn’t actually discuss with @Erik Osterman (Cloud Posse) yet, but his comment above was correct and I have closed/rejected the PR. It is a significant security risk that I do not want to detail in public.

If you want to pull a README template from some place other than Cloud Posse’s repos, you can fetch one externally in advance however you want and then set README_TEMPLATE_FILE to point to it.

irl

07:45:06 AM

Ok that implies the way we did it before with the template in the build-harness-extensions repo and setting that variable should still be working so I’ll have to dig into what’s breaking there. I’d assumed that variable didn’t work anymore since the template fetching code appeared and our setup broke at the same time.

Erik Osterman (Cloud Posse)

01:24:27 PM

Previously, the template file was cloned together with the repo. When we switched to GHE and introduced multiple github orgs, we needed a way to vary the template by org.

Erik Osterman (Cloud Posse)

01:25:03 PM

If it doesn’t find the template, it tries to download it from the organizations .github configuration repo

Erik Osterman (Cloud Posse)

01:25:56 PM

What @Jeremy G (Cloud Posse) says should work. Just modify your Makefile to specify the path to a pre existing template, and it should continue to work.

Jeremy G (Cloud Posse)

05:49:58 PM

If you export README_TEMPLATE_FILE=/path/to/file.tmpl before running make readme it should use that template. If that is not working, then I would welcome a PR to fix it, or might fix it myself if you can explain what is broken.

readme/build: readme/deps $(README_TEMPLATE_FILE) $(README_DEPS)
	@gomplate --file $(README_TEMPLATE_FILE) --out $(README_FILE) --config $(BUILD_HARNESS_PATH)/configs/gomplate.yaml && \
	echo "Generated $(README_FILE) from $(README_TEMPLATE_FILE) probably using data from $${README_YAML}"

1

irl

05:56:29 PM

I’ll have another go on Monday and see if I can pin down the issue. Thanks for the detail.

Jeremy G (Cloud Posse)

05:57:42 PM

If I had to guess, it would be that you set README_TEMPLATE_FILE to a relative path that is relative to the wrong directory.

irl

12:12:47 PM

This is a two character change.

#pr-reviews (2024-10)

2024-10-01

2024-10-02

2024-10-04

2024-10-07

2024-10-08

2024-10-09

2024-10-10

2024-10-11

2024-10-12

• Fixes <https://github.com/nodejs/undici/security/advisories/GHSA-m4v8-wqvr-p9f7 “GHSA-m4v8-wqvr-p9f7”	GHSA-m4v8-wqvr-p9f7> CVE-2024-30260
• Fixes <https://github.com/nodejs/undici/security/advisories/GHSA-9qxr-qj54-h672 “GHSA-9qxr-qj54-h672”	GHSA-9qxr-qj54-h672> CVE-2024-30261