Skip to content
This repository was archived by the owner on Mar 10, 2026. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -890,8 +890,7 @@
}
],
"source": [
"!gsutil cp {path} ."
]
"!gcloud storage cp {path} ." ]

Choose a reason for hiding this comment

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

security-medium medium

The notebook uses IPython's shell execution syntax ! with an interpolated variable {path}. Since path is derived from an external CSV file (gs://vit_models/augreg/index.csv), an attacker who can modify this CSV could inject malicious shell commands. It is recommended to quote the variable to prevent shell interpolation issues.

Suggested change
"!gcloud storage cp {path} ." ]
"!gcloud storage cp \"{path}\" ." ]

},
{
"cell_type": "code",
Expand Down
8 changes: 4 additions & 4 deletions keras_cv/src/tools/convert_presets.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ def convert_backbone_presets():
config_filename="config.json",
)
# Delete first to clean up any exising version.
os.system(f"gsutil rm -rf gs://{BUCKET}/{preset}")
os.system(f"gsutil cp -r {preset} gs://{BUCKET}/{preset}")
os.system(f"gcloud storage rm --recursive --continue-on-error gs://{BUCKET}/{preset}")
os.system(f"gcloud storage cp --recursive {preset} gs://{BUCKET}/{preset}")
Comment on lines +79 to +80

Choose a reason for hiding this comment

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

security-medium medium

The script uses os.system with f-strings containing the preset variable. If a preset name contains shell metacharacters, it could lead to command injection. While presets are currently defined within the library, using os.system with unvalidated interpolation is a dangerous pattern. It is recommended to use subprocess.run with a list of arguments, which avoids shell interpretation.

Suggested change
os.system(f"gcloud storage rm --recursive --continue-on-error gs://{BUCKET}/{preset}")
os.system(f"gcloud storage cp --recursive {preset} gs://{BUCKET}/{preset}")
subprocess.run(["gcloud", "storage", "rm", "--recursive", "--continue-on-error", f"gs://{BUCKET}/{preset}"], check=True)
subprocess.run(["gcloud", "storage", "cp", "--recursive", preset, f"gs://{BUCKET}/{preset}"], check=True)

for root, _, files in os.walk(preset):
for file in files:
path = os.path.join(BUCKET, root, file)
Expand Down Expand Up @@ -121,8 +121,8 @@ def convert_task_presets():
config_filename="config.json",
)
# Delete first to clean up any exising version.
os.system(f"gsutil rm -rf gs://{BUCKET}/{preset}")
os.system(f"gsutil cp -r {preset} gs://{BUCKET}/{preset}")
os.system(f"gcloud storage rm --recursive --continue-on-error gs://{BUCKET}/{preset}")
os.system(f"gcloud storage cp --recursive {preset} gs://{BUCKET}/{preset}")
for root, _, files in os.walk(preset):
for file in files:
path = os.path.join(BUCKET, root, file)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,6 @@
json.dump(preset_updates, f, indent=4)

print("Please run:")
print("`gsutil cp -r efficientnet_v2/ gs://keras-cv/models/`")
print("`gcloud storage cp --recursive efficientnet_v2/ gs://keras-cv/models/`")
# The gsutil to gcloud migration guide does not provide a mapping for ACL role abbreviations like 'R'.
print('`gsutil acl ch -u AllUsers:R "gs://keras-cv/models/efficientnet_v2/*"`')
Comment on lines +77 to 78

Choose a reason for hiding this comment

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

medium

The migration for gsutil acl ch seems to be incomplete here. The comment on line 77 appears to be incorrect, as other files in this pull request correctly migrate similar gsutil acl commands. Additionally, according to the gcloud documentation, the canonical syntax for granting public read access is --add-acl-grant=allUsers:READER. While AllUsers:R might work as an abbreviation, using the documented format is safer and more explicit.

Suggested change
# The gsutil to gcloud migration guide does not provide a mapping for ACL role abbreviations like 'R'.
print('`gsutil acl ch -u AllUsers:R "gs://keras-cv/models/efficientnet_v2/*"`')
print('`gcloud storage objects update "gs://keras-cv/models/efficientnet_v2/*" --add-acl-grant=allUsers:READER`')

8 changes: 4 additions & 4 deletions shell/weights/process_backbone_weights.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ echo With top checksum: $(shasum -a 256 $WEIGHTS)
echo Without top: $GCS_PATH/$OUTPUT_WEIGHTS
echo Without top checksum: $(shasum -a 256 $OUTPUT_WEIGHTS)

gsutil cp $WEIGHTS $GCS_PATH/
gsutil cp $OUTPUT_WEIGHTS $GCS_PATH/
gcloud storage cp $WEIGHTS $GCS_PATH/
gcloud storage cp $OUTPUT_WEIGHTS $GCS_PATH/

gsutil acl ch -u AllUsers:R $GCS_PATH/$WEIGHTS
gsutil acl ch -u AllUsers:R $GCS_PATH/$OUTPUT_WEIGHTS
gcloud storage objects update $GCS_PATH/$WEIGHTS --add-acl-grant=AllUsers:R
gcloud storage objects update $GCS_PATH/$OUTPUT_WEIGHTS --add-acl-grant=AllUsers:R
Comment on lines +18 to +22

Choose a reason for hiding this comment

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

security-medium medium

The script uses unquoted variables $WEIGHTS, $OUTPUT_WEIGHTS, and $GCS_PATH in shell commands, which can lead to command injection and word splitting issues. It is a security best practice to always wrap shell variables in double quotes. Additionally, for granting public read access, the canonical gcloud syntax --add-acl-grant=allUsers:READER should be used instead of AllUsers:R for clarity and safety.

Suggested change
gcloud storage cp $WEIGHTS $GCS_PATH/
gcloud storage cp $OUTPUT_WEIGHTS $GCS_PATH/
gsutil acl ch -u AllUsers:R $GCS_PATH/$WEIGHTS
gsutil acl ch -u AllUsers:R $GCS_PATH/$OUTPUT_WEIGHTS
gcloud storage objects update $GCS_PATH/$WEIGHTS --add-acl-grant=AllUsers:R
gcloud storage objects update $GCS_PATH/$OUTPUT_WEIGHTS --add-acl-grant=AllUsers:R
gcloud storage cp "$WEIGHTS" "$GCS_PATH/"
gcloud storage cp "$OUTPUT_WEIGHTS" "$GCS_PATH/"
gcloud storage objects update "$GCS_PATH/$WEIGHTS" --add-acl-grant=AllUsers:R
gcloud storage objects update "$GCS_PATH/$OUTPUT_WEIGHTS" --add-acl-grant=AllUsers:R

4 changes: 2 additions & 2 deletions shell/weights/upload_weights.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ GCS_PATH=$2

echo Checksum: $(shasum -a 256 $WEIGHTS)

gsutil cp $WEIGHTS $GCS_PATH/
gsutil acl ch -u AllUsers:R $GCS_PATH/$WEIGHTS
gcloud storage cp $WEIGHTS $GCS_PATH/
gcloud storage objects update --add-acl-grant=AllUsers:R $GCS_PATH/$WEIGHTS
Comment on lines +11 to +12

Choose a reason for hiding this comment

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

security-medium medium

The script uses unquoted variables $WEIGHTS and $GCS_PATH in shell commands, making it susceptible to command injection and word splitting. Always wrap shell variables in double quotes to prevent arbitrary command execution. Additionally, the canonical gcloud syntax --add-acl-grant=allUsers:READER should be used for granting public read access instead of AllUsers:R for better clarity and safety.

Suggested change
gcloud storage cp $WEIGHTS $GCS_PATH/
gcloud storage objects update --add-acl-grant=AllUsers:R $GCS_PATH/$WEIGHTS
gcloud storage cp "$WEIGHTS" "$GCS_PATH/"
gcloud storage objects update --add-acl-grant=AllUsers:R "$GCS_PATH/$WEIGHTS"