feat: workspace:teamId tag mapping - #19
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new --tv-tags option to enable TradingView Grype OCI label transforms (specifically mapping org.opencontainers.image.new_authors_key from HRDB-<id> to <uuid>:<id>) across the client, service, and translator components. It also adds support for loading scanner configuration overrides from INI files. The review feedback suggests improving robustness by adding error handling for INI parsing and label transforms, preventing state leakage of the tv_tags flag on reused translator instances, and handling malformed or empty label values to avoid generating empty tags.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| parser = configparser.ConfigParser() | ||
| parser.read(path) |
There was a problem hiding this comment.
Wrapping parser.read(path) in a try-except block is a good defensive programming practice. If the INI configuration file is malformed, configparser.ParsingError (or other configparser.Error subclasses) will be raised, which would crash the entire scanner client/service initialization. Handling this gracefully ensures the application remains robust.
parser = configparser.ConfigParser()
try:
parser.read(path)
except configparser.Error as e:
logger.error("Failed to parse INI config file %s: %s", config_path, e)
return {}| if label_value_transforms and k in label_value_transforms: | ||
| value_str = label_value_transforms[k](value_str) | ||
| if not value_str: | ||
| continue |
There was a problem hiding this comment.
When applying custom label value transforms, any unexpected exception raised by the transform callable (e.g., due to unexpected format or value) will crash the entire file parsing process. Wrapping the transform call in a try-except block ensures that a single malformed label does not prevent the entire scan file from being imported.
if label_value_transforms and k in label_value_transforms:
try:
value_str = label_value_transforms[k](value_str)
except Exception as e:
logger.warning("Failed to apply transform for label %s: %s", k, e)
continue
if not value_str:
continue| def _apply_tv_tags_to_translator(self, translator, tv_tags: bool) -> None: | ||
| """Enable TradingView Grype OCI label transforms on the active translator.""" | ||
| if tv_tags and hasattr(translator, "tv_tags"): | ||
| translator.tv_tags = True | ||
| logger.info("TradingView Grype tag transform enabled (--tv-tags)") |
There was a problem hiding this comment.
The _apply_tv_tags_to_translator method only sets translator.tv_tags = True if tv_tags is truthy. However, if the same translator instance is reused across multiple sequential file processing runs (e.g., in a long-lived process or when processing multiple files), a previous run with tv_tags=True will permanently leave tv_tags enabled on that translator, even for subsequent runs where tv_tags=False is requested. Explicitly setting translator.tv_tags = tv_tags prevents this state leakage.
| def _apply_tv_tags_to_translator(self, translator, tv_tags: bool) -> None: | |
| """Enable TradingView Grype OCI label transforms on the active translator.""" | |
| if tv_tags and hasattr(translator, "tv_tags"): | |
| translator.tv_tags = True | |
| logger.info("TradingView Grype tag transform enabled (--tv-tags)") | |
| def _apply_tv_tags_to_translator(self, translator, tv_tags: bool) -> None: | |
| """Enable TradingView Grype OCI label transforms on the active translator.""" | |
| if hasattr(translator, "tv_tags"): | |
| translator.tv_tags = tv_tags | |
| if tv_tags: | |
| logger.info("TradingView Grype tag transform enabled (--tv-tags)") |
| def transform_new_authors_key_value(raw: str, prefix: str) -> str: | ||
| """Transform HRDB-<id> OCI label values into <prefix>:<id> for Phoenix tags.""" | ||
| value = raw.strip() | ||
| if value.upper().startswith(HRDB_PREFIX): | ||
| value = value[len(HRDB_PREFIX):] | ||
| return f"{prefix}:{value}" |
There was a problem hiding this comment.
If the raw label value is just "HRDB-" or has trailing whitespace after the prefix (e.g., "HRDB- "), the current implementation will return "{prefix}:". This results in a malformed tag with an empty value in Phoenix. Stripping the remaining value and returning an empty string if it is empty allows promote_oci_labels to safely skip the tag instead of creating a malformed one.
| def transform_new_authors_key_value(raw: str, prefix: str) -> str: | |
| """Transform HRDB-<id> OCI label values into <prefix>:<id> for Phoenix tags.""" | |
| value = raw.strip() | |
| if value.upper().startswith(HRDB_PREFIX): | |
| value = value[len(HRDB_PREFIX):] | |
| return f"{prefix}:{value}" | |
| def transform_new_authors_key_value(raw: str, prefix: str) -> str: | |
| """Transform HRDB-<id> OCI label values into <prefix>:<id> for Phoenix tags.""" | |
| value = raw.strip() | |
| if value.upper().startswith(HRDB_PREFIX): | |
| value = value[len(HRDB_PREFIX):].strip() | |
| if not value: | |
| return "" | |
| return f"{prefix}:{value}" |
No description provided.