Skip to content

Conversation

@romanlutz
Copy link
Contributor

@romanlutz romanlutz commented Jan 1, 2026

Description

This is a follow-up to #1290 to add a minimal backend. The only APIs it includes right now are for health, version, environment variables (to choose the right ones for target configuration) and target registry-related operations. All others require a closer look in a design review (next week).

Not much changing from a frontend perspective: we now have a sidebar with the option to configure the target.
image

Note that this doesn't actually change the conversation view yet. Anything we send is simply echoed right back since the API to send messages is not included in this PR.

image image image image

Tests and Documentation

Unit tests only so far. Tests for the Typescript code to follow in another PR.

"""
Response models for API endpoints.
"""

Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't used anywhere, but I think the name responses.py is confusing. We may want to delete this to start

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's meant for response class/object definitions. I removed all the APIs for which this would have mattered from the first version, though.



@router.get("/env-vars")
async def get_env_vars() -> Dict[str, List[str]]:
Copy link
Contributor

Choose a reason for hiding this comment

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

should we keep our _async convention or want to nix it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, no harm! Didn't realize I missed it.

- exists: Whether the variable exists in environment
"""
# Don't expose API keys
if "key" in var_name.lower() or "api" in var_name.lower() or "secret" in var_name.lower():
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Let's make a function for this
  2. tricky, because I could see this going wrong for security. And also wrong by not returning averytiong (e.g. Routing_API_endpoing has api in the name)
  3. Should we have a dev version maybe? Really, it might make sense to consider all of env secrets and you just have to be super priv to call it

Copy link
Contributor

Choose a reason for hiding this comment

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

But at the very least, for now, I'd remove API

Copy link
Contributor

@rlundeen2 rlundeen2 Jan 1, 2026

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Initially I had only the env var names but it is immensely useful to see the URL and model name 🙂 if we don't want that I can remove.

"api" should have been "api_key" I suppose but that's captured by "key"

List[Dict[str, Any]]: List of target type configurations with default env vars.
"""
# Return OpenAI-compatible targets with their default environment variable names
target_types = [
Copy link
Contributor

Choose a reason for hiding this comment

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

could we have a target class attribute that fetches all targets and returns them? Maybe target_registry could do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

leaving this open, but this is further flushed out here: https://github.com/Azure/PyRIT/pull/1298/files#r2656059722

key_vars = sorted([v for v in all_vars if "KEY" in v or "SECRET" in v])

# Non-sensitive vars: don't contain KEY or SECRET
non_sensitive = [v for v in all_vars if "KEY" not in v and "SECRET" not in v]
Copy link
Contributor

@rlundeen2 rlundeen2 Jan 1, 2026

Choose a reason for hiding this comment

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

I don't love parsing env variables. It seems fragile and error prone. string matching isn't great. There's no central state. The frontend is tightly coupled to the environment. And targets are created multiple places.

I think this is a good thing to nail down early, because we will likely have similar things we tackle for Scorers, converters, etc.

Here is a proposed alternative design:

Replace fragile env var parsing with a stateful TargetRegistry singleton. The registry auto-discovers all PromptTarget subclasses on startup and attempts to instantiate them with default parameters. Users can also manually register configured instances via initializers.

TargetRegistry (singleton following CentralMemory pattern):

@dataclass
class TargetMetadata:
    """Registry metadata for a target, combining user info with intrinsic identifier."""
    id: str                          # Registry key
    name: str                        # Display name
    description: str = ""            # User-facing description
    tags: List[str] = field(default_factory=list)
    identifier: Dict[str, Any] = field(default_factory=dict)  # From get_identifier()


class TargetRegistry:
    """Singleton registry for managing prompt targets."""
    
    _instance: Optional[TargetRegistry] = None
    
    def __init__(self) -> None:
        self._targets: Dict[str, PromptTarget] = {}
        self._discovered = False
    
    @classmethod
    def get_instance(cls) -> TargetRegistry:
        if cls._instance is None:
            cls._instance = cls()
        return cls._instance
    
    @classmethod
    def reset_instance(cls) -> None:
        """Reset for testing."""
        cls._instance = None
    
    def _ensure_discovered(self) -> None:
        """Lazy auto-discovery of PromptTarget subclasses."""
        if self._discovered:
            return
        self._discover_targets()
        self._discovered = True
    
    def _discover_targets(self) -> None:
        """
        Discover all PromptTarget subclasses and attempt default instantiation.
        
        For each discovered class, tries to instantiate with no parameters.
        If successful, registers it. If it fails (missing required params),
        the class is available for manual registration but not auto-registered.
        """
        # Similar to ScenarioRegistry pattern - walk pyrit.prompt_target modules
        # For each PromptTarget subclass:
        try:
            target = TargetClass()  # Try no-arg instantiation
            self.register_target(
                target_id=self._class_to_id(TargetClass),
                target=target
            )
        except Exception:
            pass  # Requires config, skip auto-registration
    
    def register_target(
        self,
        *,
        target_id: str,
        target: PromptTarget,
        name: Optional[str] = None,
        description: str = "",
        tags: Optional[List[str]] = None
    ) -> None:
        """Register a target instance."""
        self._targets[target_id] = target
    
    def get_target(self, target_id: str) -> Optional[PromptTarget]:
        """Get a registered target by ID."""
        self._ensure_discovered()
        return self._targets.get(target_id)
    
    def list_targets(self) -> List[TargetMetadata]:
        """List all registered targets with metadata from get_identifier()."""
        self._ensure_discovered()
        return [
            TargetMetadata(
                id=target_id,
                name=name or type(target).__name__,
                target_type=type(target).__name__,
                description=description,
                endpoint=target.get_identifier().get("endpoint", ""),
                model_name=target.get_identifier().get("model_name", ""),
                tags=tags or []
            )
            for target_id, target in self._targets.items()
        ]

Then as a configurable TargetInitializer, we can add any we want. We could either have this as a server startup option, or potantially an extra "config" route (these are called on initialize_pyrit but a user could explicitly call initializers)

class AIRTTargetInitializer(PyRITInitializer):
    """Register pre-configured targets from environment variables."""
    
    @property
    def name(self) -> str:
        return "target_initializer"
    
    @property
    def required_env_vars(self) -> List[str]:
        return []  # Optional - doesn't fail if no env vars set
    
    async def initialize_async(self) -> None:
        registry = TargetRegistry.get_instance()
        
        # Register configured targets if env vars present
        if os.getenv("OPENAI_CHAT_ENDPOINT"):
            target = OpenAIChatTarget(
                endpoint=os.getenv("OPENAI_CHAT_ENDPOINT"),
                api_key=os.getenv("OPENAI_CHAT_KEY"),
                model_name=os.getenv("OPENAI_CHAT_MODEL")
            )
            registry.register_target(
                target_id="openai_chat",
                target=target,
                name="OpenAI Chat",
                description="Default OpenAI chat endpoint",
                tags=["chat", "openai"]
            )

The target routes then might look something like:

@router.get("/")
async def list_targets_async() -> List[TargetMetadata]:
    """List all registered targets."""
    return TargetRegistry.get_instance().list_targets()

@router.get("/{target_id}")
async def get_target_async(*, target_id: str) -> TargetMetadata:
    """Get specific target metadata."""
    registry = TargetRegistry.get_instance()
    target = registry.get_target(target_id)
    if not target:
        raise HTTPException(status_code=404, detail="Target not found")
    return registry.get_target_metadata(target_id)

@router.post("/")
async def create_target_async(*, request: CreateTargetRequest) -> TargetMetadata:
    """Create and register a new target instance."""
    registry = TargetRegistry.get_instance()
    target = registry.create_from_type(
        target_type=request.target_type,
        **request.config
    )
    registry.register_target(
        target_id=request.target_id,
        target=target,
        name=request.name,
        description=request.description,
        tags=request.tags
    )
    return registry.get_target_metadata(request.target_id)

Steps might look like (AI generated)

  1. Create TargetMetadata dataclass in models - typed metadata with id, name, description, tags, identifier.
  2. Refactor TargetRegistry in target_registry.py - singleton with _instance, lazy _discover_targets(), register_target(), get_target(), list_targets().
  3. Add _discover_targets() method - walk pyrit.prompt_target submodules (like ScenarioRegistry), try TargetClass() instantiation, register successes, skip failures.
  4. Create TargetInitializer in initializers (or two) - registers env-var-configured targets on startup.
  5. Delete config.py and remove from main.py.
  6. Simplify targets.py - list_targets_async(), get_target_async(), create_target_async() calling registry.
  7. Add test fixture - TargetRegistry.reset_instance() called in pytest fixture for isolation.

Copy link
Contributor

@rlundeen2 rlundeen2 Jan 1, 2026

Choose a reason for hiding this comment

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

Also note,

  • It might make sense to do a separate TargetRegistry PR vs lumping it in
  • TargetRegistry will likely need some locking
  • I like the idea of eventually loading these from the db so we don't lose them on a server restart. But even in that case, I love having initializers because some users will want to use it locally and I think that makes it easy to get started. Imo we should punt db loading at first
  • Consider putting scenarioRegistry, InitializerRegistry, and this together. The Targetregistey would be super nice for the cli too because then you could use it to more easily select objective targets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes "adding a target" an entire process, though. Before, it was as easy as changing a secrets file (no PR) while it would now be a code change for PyRIT to register the target. Assume that most users just want to use this via Docker (setup will be in a separate PR) so now they have to make code changes inside the container rather than just updating a env file that's passed in. That is a massive extra effort (unless you see some shortcut). I think that's generally something we'll need to sort out with docker+initializers for scenarios.

I'm no fan of the current env var parsing either but the point was to get a POC for the GUI and this was easily the fastest way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Assume we tried the design I proposed and think about it from the user perspective.

Even with only an env file it will load default targets.

But we could pre provide initializers (or reuse existing). For example, one initializer we could write could try to load every env var in our .env_example. Another could even do the same env parsing that you have in the PR. It doesn't need programming from users. They'd just provide which initializer when they start the server, or they could load one with another rest backend call (eg run initializer with /initialize). This might be nice anyway to set different types of things via initializers (eg loading default datasets or setting the backend config).

Separately via the /targets call they can also create their own targets which add to target registry.

If we separate it this way, we decouple the configuration and have more flexibility.

Sign up for free to 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.

2 participants