feat: strip plugin fields marked as secret before model_dump()

[red40maxxer]

I'm implementing an LLM refusal judge plugin that uses an OAI-compatible API, and obviously the API key shouldn't included in the reproduce.json. Pydantic already provides a way to mark fields as secret with SecretStr and SecretBytes, but we can't just use these in the settings fields directly. model_dump() dumps the parent Settings which will include the model_extra that contains our raw, untyped scorer settings, so Pydantic can't automatically redact it (since there's no type information). Also, if there was a typed SecretStr field on Settings, tomli_w gets handed an unserializable SecretStr object and errors

Still a WIP, there are many other design decisions to figure out but happy to have this roasted, this is probably going to be a hard requirement as I forsee lots of people making LLM judge plugins and leaking their API keys everywhere

[gemini-code-assist]

Code Review

This pull request introduces functionality to strip sensitive plugin settings (typed with Pydantic's Secret, SecretStr, or SecretBytes) from serialized outputs like config.toml and reproduce.json. The review feedback points out a critical issue where secret fields wrapped in Union, Optional, or Annotated types are not correctly identified, potentially leading to credential leaks. Additionally, the feedback addresses style guide violations regarding the use of abbreviations and comment formatting.

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.

[gemini-code-assist]

The current implementation of secret_field_names only checks if field.annotation is a direct subclass of Secret, SecretStr, or SecretBytes. However, if a field is optional (e.g., Optional[SecretStr] or SecretStr | None) or annotated (e.g., Annotated[SecretStr, Field(...)]), field.annotation will be a generic alias or union type rather than a direct subclass. This causes secret fields to be silently ignored and not stripped, potentially leaking sensitive credentials.

We should recursively unwrap the type annotation to handle Union, Optional, and Annotated types correctly.

Suggested change

	for name, field in model.model_fields.items():
	annotation = field.annotation
	is_secret_type = isinstance(annotation, type) and issubclass(
	annotation, (Secret, SecretStr, SecretBytes)
	)
	if is_secret_type:
	names.add(name)
	from typing import get_origin, get_args, Annotated, Union
	import types
	def is_secret_type(tp: Any) -> bool:
	if tp is None:
	return False
	origin = get_origin(tp)
	if origin is Annotated:
	return is_secret_type(get_args(tp)[0])
	if origin in (Union, getattr(types, "UnionType", None)):
	return any(is_secret_type(arg) for arg in get_args(tp))
	base_type = origin if origin is not None else tp
	return isinstance(base_type, type) and issubclass(
	base_type, (Secret, SecretStr, SecretBytes)
	)
	for name, field in model.model_fields.items():
	if is_secret_type(field.annotation):
	names.add(name)

[gemini-code-assist]

This block contains a few style guide violations:

1. The identifier cfg is an abbreviation, which violates the repository style guide rule against abbreviations. We should rename it to scorer_config.
2. The comment on line 95 (# Kind of ugly, needs a second thought here) and the comment on line 103 (# TODO: think about removing vs placeholder) do not end with a period, and the latter does not start with a capital letter.

Let's refactor this block to resolve these style guide violations.

    for scorer_config in settings.scorers:
        plugin_cls = load_plugin(scorer_config.plugin, Scorer)
        model = plugin_cls.get_settings_model()
        if model is None:
            continue
        
        secrets = secret_field_names(model)
        if not secrets:
            continue
        
        class_name = plugin_cls.__name__
        candidates = [class_name]
        # This can be refactored to be cleaner in a future iteration.
        if scorer_config.instance_name:
            candidates.append(f"{class_name}_{scorer_config.instance_name}")
        
        for table_name in candidates:
            table = scorer_tables.get(table_name)
            if isinstance(table, dict):
                for field in secrets:
                    # TODO: Think about removing vs placeholder.
                    table.pop(field, None)

References

1. Identifier names should not contain abbreviations unless those abbreviations are very widely used and understood (e.g. "KL divergence"). ^(link)
2. Comments should start with a capital letter and end with a period. They should use correct grammar and spelling. ^(link)

[gemini-code-assist]

The comment here does not start with a capital letter and does not end with a period, which violates the repository style guide. Let's capitalize the first word and add a period at the end.

Suggested change

	# TODO: we might have to warn the user here that this might not be directly reproducible,
	# as the plugin contains secret fields that will be omitted from the final `reproduce.json`.
	# TODO: We might have to warn the user here that this might not be directly reproducible,
	# as the plugin contains secret fields that will be omitted from the final `reproduce.json`.

References

1. Comments should start with a capital letter and end with a period. They should use correct grammar and spelling. ^(link)

[p-e-w]

I faced a similar problem when designing the headless mode, and I came to this conclusion:

heretic/src/heretic/main.py

Lines 1062 to 1069 in c8a254b

	# NOTE: Unlike for most other values obtained from interactive inputs, it is
	# not possible to set the token via the settings. This is a security
	# precaution to prevent exporting the token under all circumstances.
	# For scripting, the correct way to set the token is through the HF_TOKEN
	# environment variable, or through the HF token file.
	token = questionary.password(
	"Hugging Face access token:"
	).ask()

Relying on persistence exclusion mechanisms is always brittle. The industry standard for handling secrets is environment variables, and all LLM client libraries support them out of the box. The only secure solution is to not store such things in configuration files at all, because even if the reproducibility mode takes care of it somehow, people might still share config files directly etc.

[p-e-w]

I also thought about this for the upcoming logger plugins, where we will support pushing to Weights and Biases.

The solution is again the same: The official wandb library supports passing the key through the WANDB_API_KEY environment variable, or automatic pickup when logging in locally with wandb login. This approach works everywhere, and we never have to worry about leaking secrets in the first place.

[p-e-w]

As for the real problem of plugins potentially leaking sensitive information (not just API secrets, but also details like local paths):

I propose that plugins have to opt in to reproducibility. They need to set a flag to true in order to be serialized, and if any loaded plugins don't have that flag set, we can't reproduce. Deciding whether to set this flag is the responsibility of the plugin author, and it is documented what this entails (serialization + publishing).

[red40maxxer]

I propose that plugins have to opt in to reproducibility. They need to set a flag to true in order to be serialized, and if any loaded plugins don't have that flag set, we can't reproduce. Deciding whether to set this flag is the responsibility of the plugin author, and it is documented what this entails (serialization + publishing).

I think this can be done cleanly by making it a class-level property same as score_name - if it was in the settings TOML, we'd need to add special case filtering logic in get_settings_model to make sure users can't override it. Do you want this to be manually done by the author or should there be something that auto-derives it if fields are marked as Secret? I'd rather go with the first approach and not even deal with Secret at all tbh, that seems like the simplest and cleanest solution to me

[p-e-w]

Agreed, this should be a property of the plugin itself, not a configuration. The author needs to explicitly set the reproducible property on the class, otherwise the system assumes that runs done with the plugin loaded are not reproducible. Important subtlety: The property is relevant even if the plugin is configured with optimization set to none, because if the plugin behaves non-deterministically internally, it will thwart the global RNG state that other parts of the system rely on for reproducibility.

As explained above, I don't think Heretic should have any special handling for secrets. It just creates a false sense of security when we don't really intend to provide strong guarantees, and all relevant libraries are able to handle secrets through external mechanisms.