-
-
Notifications
You must be signed in to change notification settings - Fork 809
feat(redteam): hydrate applicationDefinition when importing YAML configs #6542
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Previously, importing a YAML config would only preserve the `purpose` string, losing all 18 structured applicationDefinition fields (redteamUser, accessToData, forbiddenData, etc.). This made re-importing configs created via the UI lose important context. Changes: - Add `purposeToApplicationDefinition()` parser that extracts structured fields from the formatted purpose string generated by the UI - Update `handleFileUpload()` to use the parser with fallback chain: 1. Explicit `applicationDefinition` in YAML (future-proof) 2. Parse from `purpose` string using section headers 3. Fall back to treating entire string as purpose field - Improve import discoverability with hint on Target Type Selection page - Enhance Load Config dialog with better titles and prominent import button - Import additional YAML fields: testGenerationInstructions, language, extensions The parser handles: - All 18 applicationDefinition fields with exact header matching - Multiline content within code blocks - Special characters (quotes, brackets, ampersands) - Plain text fallback for configs not created via UI - Empty/null/undefined inputs safely Closes #6493 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
⏩ No tests generated (5f10079) View output ↗ Existing test issuesTests generated in these files will include these fixes. If you wish to only commit these fixes, you can do so in the Tusk app. Tests for these symbols were not generated because the existing test files had errors and were failed to fix:
The issues in these test files must be resolved before Tusk can generate tests for these symbols. Please contact Support if you have questions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 All Clear
I reviewed this PR for LLM security vulnerabilities. The changes add configuration parsing utilities and UI improvements for importing YAML configs in the red team setup interface. No LLM-related code changes were found - this PR only contains text parsing logic, UI components, and test files.
Minimum severity threshold for this scan: 🟡 Medium | Learn more
📝 WalkthroughWalkthroughThe changes implement YAML file import functionality for the redteaming setup workflow. A new utility module purposeParser.ts converts structured purpose strings into ApplicationDefinition objects with parsing logic and fallback handlers. The setup page is enhanced with UI for importing YAML configurations and displaying saved configurations. A comprehensive test suite validates the parser behavior across various scenarios. The Quick Start alert UI is restructured to accommodate the expanded import workflow. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/src/pages/redteam/setup/page.tsx (1)
491-525: Avoid mutatingDEFAULT_HTTP_TARGETand remove leftover debug loggingIn
handleFileUpload, the fallback:let target = yamlConfig.targets?.[0] || yamlConfig.providers?.[0] || DEFAULT_HTTP_TARGET; ... if (hasAnyStatefulStrategies) { if (typeof target === 'string') { target = { id: target, config: { stateful: true } }; } else { target.config = { ...target.config, stateful: true }; } }can end up mutating
DEFAULT_HTTP_TARGET.configwhen no targets/providers are present in the YAML, sincetargetwill reference the shared default object. That leaks thestatefulflag into future configs that rely on the default, which is hard to trace.There’s also a
console.logthat looks like temporary debugging.A minimal fix is to clone the default when used and drop the debug log:
- const strategies = yamlConfig?.redteam?.strategies || []; - let target = yamlConfig.targets?.[0] || yamlConfig.providers?.[0] || DEFAULT_HTTP_TARGET; + const strategies = yamlConfig?.redteam?.strategies || []; + let target = + yamlConfig.targets?.[0] || yamlConfig.providers?.[0] || { ...DEFAULT_HTTP_TARGET }; @@ - const hasAnyStatefulStrategies = strategies.some( - (strat: RedteamStrategy) => typeof strat !== 'string' && strat?.config?.stateful, - ); - console.log({ hasAnyStatefulStrategies, strategies }); + const hasAnyStatefulStrategies = strategies.some( + (strat: RedteamStrategy) => typeof strat !== 'string' && strat?.config?.stateful, + );This keeps
DEFAULT_HTTP_TARGETimmutable while preserving the intended behavior.
🧹 Nitpick comments (5)
src/app/src/pages/redteam/setup/utils/purposeParser.test.ts (2)
369-493: Avoid duplicatingapplicationDefinitionToPurposeimplementation in testsRe‑implementing the serializer inline in the tests risks drift if the production
applicationDefinitionToPurposeformat changes (e.g., header text, ordering, or new sections). Consider extracting the shared serialization logic into a small, dependency‑free utility module that bothuseRedTeamConfigand these tests can import, so the tests always exercise the exact production format.
495-593: Optional: add at least one whole-object assertion for roundtrip verificationIn the roundtrip tests, you assert each field individually. For future additions to
ApplicationDefinition, you could complement this with a single assertion that compares the parsed result to the original object (or a fixture) as a whole, so new fields are automatically checked without expanding per-field expectations.src/app/src/pages/redteam/setup/page.tsx (2)
526-547: applicationDefinition hydration priority looks correct; consider normalizing explicit YAML definitionsThe precedence of
applicationDefinition(explicit YAML → parsed frompurpose→ empty defaults via the parser) matches the PR intent and should restore structured fields for older YAMLs that only have a purpose block.If you expect
redteam.applicationDefinitionfrom YAML to sometimes be partial or loosely shaped, you may want to normalize it with the same defaulting strategy used elsewhere (e.g., viamergeWithDefaults) so the form always sees a fully-populatedApplicationDefinitionobject. Not strictly required for correctness here, but it would give downstream code a more predictable shape.
51-52: Optional: align new import with@app/*alias conventionThe new
purposeToApplicationDefinitionimport uses a relative path, while the project’s newer code tends to favor@app/...aliases undersrc/app/src. When convenient, consider updating this (and potentially neighboring imports) to the alias form for consistency.src/app/src/pages/redteam/setup/utils/purposeParser.ts (1)
9-54: SECTION_MAPPINGS and defaults align with ApplicationDefinition, but headers should ideally be centralizedThe mappings and
DEFAULT_APPLICATION_DEFINITIONcover all the structured fields you’re parsing from the purpose string (excludingsystemPrompt, which isn’t represented in these headers). This is consistent with the test data and serializer format.Because the comments note these headers are generated by
applicationDefinitionToPurpose()inuseRedTeamConfig.ts, consider centralizing the header strings in a shared constants module used by both the serializer and this parser. That would prevent subtle bugs if the UI copy for a section header ever changes in one place but not the other.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsx(1 hunks)src/app/src/pages/redteam/setup/page.tsx(6 hunks)src/app/src/pages/redteam/setup/utils/purposeParser.test.ts(1 hunks)src/app/src/pages/redteam/setup/utils/purposeParser.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/app/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/app/AGENTS.md)
src/app/src/**/*.{ts,tsx}: NEVER usefetch()directly. ALWAYS usecallApi()from '@app/utils/api' for API calls to handle dev/prod API URL differences
Avoid unnecessary memoization with useMemo() unless performance is verified as a problem
Use @app/* path alias (maps to src/*) configured in vite.config.ts for all imports
Files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.tssrc/app/src/pages/redteam/setup/utils/purposeParser.tssrc/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsxsrc/app/src/pages/redteam/setup/page.tsx
src/app/src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (src/app/AGENTS.md)
src/app/src/**/*.test.{ts,tsx}: Import mocking utilities from 'vitest' (describe, it, expect, vi, beforeEach, afterEach) for test files
Use @testing-library/react (render, screen, fireEvent) for component testing instead of Jest utilities
Mock the '@app/utils/api' module and callApi function in tests using vi.mock()
Files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.ts
src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/app/**/*.{ts,tsx}: When making API calls from the React app, use thecallApifunction from@app/utils/apiinstead of directfetch()calls to ensure proper API base URL handling
UseuseMemowhen computing a value that doesn't accept arguments (a non-callable) in React hooks
UseuseCallbackwhen creating a stable function reference that accepts arguments and will be called later in React hooks
Files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.tssrc/app/src/pages/redteam/setup/utils/purposeParser.tssrc/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsxsrc/app/src/pages/redteam/setup/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict type checking
Files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.tssrc/app/src/pages/redteam/setup/utils/purposeParser.tssrc/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsxsrc/app/src/pages/redteam/setup/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Follow consistent import order using Biome for import sorting
Use consistent curly braces for all control statements
Prefer const over let; avoid var
Use object shorthand syntax whenever possible
Use async/await for asynchronous code
Use consistent error handling with proper type checks
Always sanitize sensitive data before logging to prevent exposing secrets, API keys, passwords, and other credentials in logs
Use the sanitized logging method by passing context objects as a second parameter to logger methods (debug,info,warn,error) which will automatically sanitize sensitive fields
For manual sanitization outside of logging contexts, use thesanitizeObjectfunction from./util/sanitizerwhich sanitizes recursively up to 4 levels deep
Files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.tssrc/app/src/pages/redteam/setup/utils/purposeParser.tssrc/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsxsrc/app/src/pages/redteam/setup/page.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for all tests in both backend tests in
test/and frontend tests insrc/app/
Files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.ts
🧠 Learnings (16)
📚 Learning: 2025-11-29T00:26:16.694Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/redteam/AGENTS.md:0-0
Timestamp: 2025-11-29T00:26:16.694Z
Learning: Applies to src/redteam/test/redteam/**/*.ts : Add tests for new red team plugins in the `test/redteam/` directory
Applied to files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.tssrc/app/src/pages/redteam/setup/page.tsx
📚 Learning: 2025-11-29T00:26:16.694Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/redteam/AGENTS.md:0-0
Timestamp: 2025-11-29T00:26:16.694Z
Learning: Applies to src/redteam/plugins/*.ts : Include assertions defining failure conditions in red team plugin test cases
Applied to files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.tssrc/app/src/pages/redteam/setup/page.tsx
📚 Learning: 2025-12-01T18:19:09.570Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: test/AGENTS.md:0-0
Timestamp: 2025-12-01T18:19:09.570Z
Learning: Applies to test/providers/**/*.test.{ts,tsx,js} : Provider tests must cover: success case (normal API response), error cases (4xx, 5xx, rate limits), configuration validation, and token usage tracking
Applied to files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.tssrc/app/src/pages/redteam/setup/page.tsx
📚 Learning: 2025-12-01T18:19:09.570Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: test/AGENTS.md:0-0
Timestamp: 2025-12-01T18:19:09.570Z
Learning: Applies to test/**/*.test.{ts,tsx,js} : Organize tests with nested `describe()` and `it()` blocks to structure test suites logically
Applied to files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.ts
📚 Learning: 2025-11-29T00:26:16.694Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/redteam/AGENTS.md:0-0
Timestamp: 2025-11-29T00:26:16.694Z
Learning: Applies to src/redteam/plugins/*.ts : Generate targeted test cases for specific vulnerabilities in red team plugins
Applied to files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.tssrc/app/src/pages/redteam/setup/page.tsx
📚 Learning: 2025-12-01T18:19:09.570Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: test/AGENTS.md:0-0
Timestamp: 2025-12-01T18:19:09.570Z
Learning: Applies to test/**/*.test.{ts,tsx,js} : Test entire objects with `expect(result).toEqual({...})` rather than individual fields
Applied to files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.ts
📚 Learning: 2025-12-01T18:18:45.551Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/app/AGENTS.md:0-0
Timestamp: 2025-12-01T18:18:45.551Z
Learning: Applies to src/app/src/**/*.test.{ts,tsx} : Import mocking utilities from 'vitest' (describe, it, expect, vi, beforeEach, afterEach) for test files
Applied to files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.tssrc/app/src/pages/redteam/setup/page.tsx
📚 Learning: 2025-07-18T17:25:57.700Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: .cursor/rules/gh-cli-workflow.mdc:0-0
Timestamp: 2025-07-18T17:25:57.700Z
Learning: Applies to **/*.{test,spec}.{js,ts,jsx,tsx} : Avoid disabling or skipping tests unless absolutely necessary and documented
Applied to files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.ts
📚 Learning: 2025-10-06T03:43:01.653Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-06T03:43:01.653Z
Learning: Applies to test/**/*.{ts,tsx,js,jsx} : Follow Jest best practices using describe and it blocks in tests
Applied to files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.ts
📚 Learning: 2025-12-04T23:54:13.885Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T23:54:13.885Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use Vitest for all tests in both backend tests in `test/` and frontend tests in `src/app/`
Applied to files:
src/app/src/pages/redteam/setup/utils/purposeParser.test.ts
📚 Learning: 2025-11-29T00:24:17.021Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/redteam/CLAUDE.md:0-0
Timestamp: 2025-11-29T00:24:17.021Z
Learning: Applies to src/redteam/**/*agent*.{ts,tsx,js,jsx} : Maintain clear agent interface definitions and usage patterns
Applied to files:
src/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsxsrc/app/src/pages/redteam/setup/page.tsx
📚 Learning: 2025-11-29T00:25:25.321Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: site/docs/AGENTS.md:0-0
Timestamp: 2025-11-29T00:25:25.321Z
Learning: Applies to site/docs/**/*.md : Use 'Promptfoo' (capitalized) at the start of sentences and headings, and 'promptfoo' (lowercase) in code, commands, and package names
Applied to files:
src/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsx
📚 Learning: 2025-11-29T00:25:12.643Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: examples/AGENTS.md:0-0
Timestamp: 2025-11-29T00:25:12.643Z
Learning: Applies to examples/**/promptfooconfig.yaml : Keep description field SHORT (3-10 words)
Applied to files:
src/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsx
📚 Learning: 2025-12-01T18:18:56.517Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/providers/AGENTS.md:0-0
Timestamp: 2025-12-01T18:18:56.517Z
Learning: Applies to src/providers/**/*.ts : Transform promptfoo prompts into provider-specific API format and return normalized `ProviderResponse` for evaluation
Applied to files:
src/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsx
📚 Learning: 2025-12-01T18:18:45.551Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/app/AGENTS.md:0-0
Timestamp: 2025-12-01T18:18:45.551Z
Learning: Applies to src/app/src/components/**/*.{ts,tsx} : Use icons from mui/icons-material for UI icons
Applied to files:
src/app/src/pages/redteam/setup/page.tsx
📚 Learning: 2025-12-01T18:18:45.551Z
Learnt from: CR
Repo: promptfoo/promptfoo PR: 0
File: src/app/AGENTS.md:0-0
Timestamp: 2025-12-01T18:18:45.551Z
Learning: Applies to src/app/src/**/*.test.{ts,tsx} : Use testing-library/react (render, screen, fireEvent) for component testing instead of Jest utilities
Applied to files:
src/app/src/pages/redteam/setup/page.tsx
🧬 Code graph analysis (4)
src/app/src/pages/redteam/setup/utils/purposeParser.test.ts (2)
src/app/src/pages/redteam/setup/utils/purposeParser.ts (3)
purposeToApplicationDefinition(80-141)hasApplicationDefinitionContent(146-152)mergeWithDefaults(157-164)src/app/src/pages/redteam/setup/types.ts (1)
ApplicationDefinition(4-25)
src/app/src/pages/redteam/setup/utils/purposeParser.ts (1)
src/app/src/pages/redteam/setup/types.ts (1)
ApplicationDefinition(4-25)
src/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsx (1)
src/app/src/pages/redteam/setup/components/LoadExampleButton.tsx (1)
LoadExampleButton(12-51)
src/app/src/pages/redteam/setup/page.tsx (1)
src/app/src/pages/redteam/setup/utils/purposeParser.ts (1)
purposeToApplicationDefinition(80-141)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: Tusk Test Runner (3)
- GitHub Check: Tusk Test Runner (4)
- GitHub Check: Tusk Test Runner (1)
- GitHub Check: Tusk Test Runner (2)
- GitHub Check: Tusk Tester
- GitHub Check: webui tests
- GitHub Check: Share Test
- GitHub Check: Build Docs
- GitHub Check: Redteam (Staging API)
- GitHub Check: Redteam (Production API)
- GitHub Check: Test on Node 24.x and windows-latest
- GitHub Check: Test on Node 22.x and windows-latest
- GitHub Check: Test on Node 24.x and ubuntu-latest
- GitHub Check: Test on Node 22.x and macOS-latest
- GitHub Check: Test on Node 20.x and macOS-latest
- GitHub Check: Test on Node 22.x and ubuntu-latest
- GitHub Check: Test on Node 20.x and windows-latest
- GitHub Check: Test on Node 20.x and ubuntu-latest
- GitHub Check: Build on Node 22.x
- GitHub Check: Build on Node 24.x
- GitHub Check: Build on Node 20.x
- GitHub Check: security-scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
src/app/src/pages/redteam/setup/components/Targets/TargetTypeSelection.tsx (1)
150-181: Quick-start layout and copy align with new YAML import flowThe Stack/Box restructuring and updated copy clearly explain both loading the example and importing YAML via the “Load Config” sidebar button, without affecting any behavior. Styling change on the alert icon is also safe.
src/app/src/pages/redteam/setup/utils/purposeParser.test.ts (1)
1-308: Thorough coverage of parser edge cases and real-world formatsThe suite exercises all major behaviors (structured sections, fallbacks, malformed input, partial matches, and real-world examples) plus the helper utilities and roundtrip flows. This gives good confidence that YAML-imported purpose strings will be interpreted correctly.
src/app/src/pages/redteam/setup/utils/purposeParser.ts (1)
80-164: Parser and helper behavior match the documented fallbacks
purposeToApplicationDefinitioncorrectly:
- returns empty defaults for undefined/empty/non-string inputs,
- parses the primary code-block format,
- falls back to the non–code block format, and
- finally treats the entire string as
purposewhen no known sections are found.
hasApplicationDefinitionContentandmergeWithDefaultsare straightforward and do what their names imply. No functional issues spotted here.
e04e97e to
5f10079
Compare
Summary
purposestring was preserved; now all 18 fields (redteamUser, accessToData, forbiddenData, etc.) are parsed and hydrated back into the formChanges
New: Purpose Parser (
purposeParser.ts)Parses the formatted purpose string back into structured
ApplicationDefinitionfields:Updated: Import Handler (
page.tsx)Priority chain for hydrating applicationDefinition:
applicationDefinitionin YAML (future-proof)purposestring using section headersAlso imports additional fields:
testGenerationInstructions,language,extensionsUX Improvements
Test plan
Closes #6493
🤖 Generated with Claude Code