-
-
Notifications
You must be signed in to change notification settings - Fork 854
feat(model-audit): add auto-sharing with --share and --no-share flags #6928
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
Conversation
Add automatic sharing functionality to model audits with the same behavior as eval sharing: - Add --share and --no-share CLI flags to scan-model command - Auto-share by default when cloud is enabled (matches eval behavior) - Non-blocking: results display immediately, sharing happens in background - Respect PROMPTFOO_DISABLE_SHARING environment variable - Server-side deduplication handles duplicate uploads automatically Implementation details: - Update ScanOptions interface with share/noShare properties - Add sharing decision logic following eval precedence hierarchy - Start upload immediately in background (non-blocking like evals) - Display results immediately without waiting for upload - Show spinner with shareable URL after upload completes - Update model audit documentation with sharing examples The implementation provides the best developer experience by showing scan results immediately while sharing happens in the background, exactly matching eval sharing behavior.
|
⏩ No test execution environment matched (7f96a5c) View output ↗ View check history
|
Security Review: PR #6928No critical security issues found. Reviewed for injection vulnerabilities, unsafe data handling, credential exposure, and logic errors. The implementation follows the existing eval sharing pattern and maintains proper security boundaries. Testing Issue - Missing Test Coverage
PR Convention Check
Code Quality Notes - Good patterns observed:
Security boundaries maintained:
|
📝 WalkthroughWalkthroughThis pull request introduces a sharing feature for model audit results. It adds documentation explaining the auto-sharing behavior when connected to promptfoo Cloud, including CLI flags (--share, --no-share) and an environment variable (PROMPTFOO_DISABLE_SHARING) to control sharing. The implementation in modelScan.ts determines whether to share based on explicit flags or cloud defaults, initiates background uploading of audit data, and provides environment-aware feedback (spinner UI in TTY environments, direct logging in CI). The CLI parser is updated to define and handle the new share and noShare options. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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: 1
Fix all issues with AI Agents 🤖
In @src/commands/modelScan.ts:
- Around line 538-558: The sharing gate currently prevents calling
createShareableModelAuditUrl unless cloudConfig.isEnabled() is true, which
blocks explicit --share; change the runtime check so that once wantsToShare is
true (derived from options.share and hasExplicitDisable), you call
createShareableModelAuditUrl(audit) regardless of cloudConfig.isEnabled(); in
other words, replace the conditional "if (wantsToShare &&
cloudConfig.isEnabled())" with "if (wantsToShare)" so
createShareableModelAuditUrl can perform its own fallback to
getShareApiBaseUrl().
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
site/docs/model-audit/usage.mdsrc/commands/modelScan.tssrc/util/modelAuditCliParser.ts
🧰 Additional context used
📓 Path-based instructions (5)
site/docs/**/*.md
📄 CodeRabbit inference engine (site/AGENTS.md)
site/docs/**/*.md: Don't modify heading text in documentation (often externally linked)
Avoid embellishment words like 'sophisticated' in documentation
Include front matter with title (under 60 chars), description (150-160 chars), and sidebar_position on all documentation pages
Addtitle="filename.yaml"attribute only to code blocks containing complete, runnable files
Don't add titles to code block fragments (only complete, runnable files)
Use// highlight-next-linedirective to emphasize specific lines in code blocks
Never remove existing highlight directives from code blocks
Use admonition syntax (:::note, :::warning, :::danger) with empty lines around content
Files:
site/docs/model-audit/usage.md
site/**/*.{md,ts,tsx,js,json}
📄 CodeRabbit inference engine (site/AGENTS.md)
site/**/*.{md,ts,tsx,js,json}: Use 'eval' not 'evaluation' in commands and documentation
Use 'Promptfoo' when referring to the company or product, 'promptfoo' when referring to the CLI command or in code
Files:
site/docs/model-audit/usage.md
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict type checking
Files:
src/util/modelAuditCliParser.tssrc/commands/modelScan.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Follow consistent import order (Biome handles sorting)
Use consistent curly braces for all control statements
Preferconstoverlet; avoidvar
Use object shorthand syntax whenever possible
Useasync/awaitfor asynchronous code
Use consistent error handling with proper type checks
Avoid re-exporting from files; import directly from the source module
Use the logger with object context (auto-sanitized)
Files:
src/util/modelAuditCliParser.tssrc/commands/modelScan.ts
src/commands/**/*.ts
📄 CodeRabbit inference engine (src/commands/AGENTS.md)
src/commands/**/*.ts: Register CLI commands with Commander insrc/main.ts
UsesetupEnv()for environment setup in CLI commands
Track command usage withtelemetry.record()
Useloggerfor all output messages, never useconsolefor logging in CLI commands
Setprocess.exitCode = 1on errors instead of usingprocess.exit()
Validate command inputs and arguments with Zod
Files:
src/commands/modelScan.ts
🧬 Code graph analysis (2)
src/util/modelAuditCliParser.ts (1)
src/providers/litellm.ts (1)
config(41-43)
src/commands/modelScan.ts (3)
src/envars.ts (2)
getEnvBool(441-450)isCI(520-536)src/globalConfig/cloud.ts (1)
cloudConfig(250-250)src/share.ts (1)
createShareableModelAuditUrl(593-666)
⏰ 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). (26)
- GitHub Check: Test on Node 20.x and windows-latest (shard 2/3)
- GitHub Check: Test on Node 22.x and ubuntu-latest
- GitHub Check: Test on Node 24.x and windows-latest (shard 2/3)
- GitHub Check: Test on Node 24.x and windows-latest (shard 3/3)
- GitHub Check: Test on Node 22.x and macOS-latest
- GitHub Check: Test on Node 24.x and ubuntu-latest
- GitHub Check: Redteam (Staging API)
- GitHub Check: Test on Node 20.x and ubuntu-latest
- GitHub Check: Test on Node 20.x and macOS-latest
- GitHub Check: Test on Node 22.x and windows-latest (shard 3/3)
- GitHub Check: Test on Node 22.x and windows-latest (shard 2/3)
- GitHub Check: Test on Node 24.x and windows-latest (shard 1/3)
- GitHub Check: Test on Node 20.x and windows-latest (shard 1/3)
- GitHub Check: Test on Node 20.x and windows-latest (shard 3/3)
- GitHub Check: webui tests
- GitHub Check: Test on Node 22.x and windows-latest (shard 1/3)
- GitHub Check: Share Test
- GitHub Check: Redteam (Production API)
- GitHub Check: Build Docs
- GitHub Check: Build on Node 22.x
- GitHub Check: Build on Node 20.x
- GitHub Check: Style Check
- GitHub Check: claude-review
- GitHub Check: Build on Node 24.x
- GitHub Check: security-scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
site/docs/model-audit/usage.md (1)
206-231: LGTM - Documentation clearly explains the sharing feature.The documentation accurately reflects the implementation: auto-sharing when cloud is enabled, CLI flags for explicit control, and environment variable for global disable. The examples are practical and cover the common use cases.
src/commands/modelScan.ts (4)
9-17: LGTM - Imports support the new sharing functionality.The new imports are properly scoped and all are used in the sharing implementation below.
64-66: LGTM - Interface correctly extended for sharing options.
575-606: LGTM - Good environment-aware sharing feedback.The TTY vs CI detection provides appropriate UX: spinner with progress in interactive terminals, simple log output in CI. The non-blocking design matches the documented behavior.
One minor note: Share errors are logged at
debuglevel (lines 593, 603). Users won't see share failures unless running with verbose mode. This may be intentional to avoid noise, but consider if awarnlevel would be more appropriate for visibility.
741-744: LGTM - CLI options properly defined.The
--shareand--no-shareflags are correctly registered. Commander will handle--no-shareby settingnoShare: truein the options object.src/util/modelAuditCliParser.ts (3)
37-41: LGTM - Schema correctly extended with promptfoo-only options.The
shareandnoShareoptions are properly typed as optional booleans with a clear comment indicating they are not passed to modelaudit.
101-128: LGTM - Type change supports intentional omissions.Using
Partial<Record<...>>correctly allows theshareandnoSharekeys to be absent from the mapping. The comment on line 103 documents this design decision well.
143-145: LGTM - Correctly handles missing CLI_ARG_MAP entries.The
!configcheck properly skips options that are intentionally not mapped (likeshareandnoShare), preventing runtime errors when iterating over the Partial record.
src/commands/modelScan.ts
Outdated
| // Determine if we should share | ||
| const hasExplicitDisable = | ||
| options.noShare === true || getEnvBool('PROMPTFOO_DISABLE_SHARING'); | ||
|
|
||
| let wantsToShare: boolean; | ||
| if (hasExplicitDisable) { | ||
| wantsToShare = false; | ||
| } else if (options.share === true) { | ||
| wantsToShare = true; | ||
| } else { | ||
| // Default: auto-share when cloud is enabled | ||
| wantsToShare = cloudConfig.isEnabled(); | ||
| } | ||
|
|
||
| logger.debug(`Model audit sharing decision: wantsToShare=${wantsToShare}`); | ||
|
|
||
| // Start sharing in background (don't await yet - non-blocking like evals!) | ||
| let sharePromise: Promise<string | null> | null = null; | ||
| if (wantsToShare && cloudConfig.isEnabled()) { | ||
| sharePromise = createShareableModelAuditUrl(audit); | ||
| } |
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.
Sharing condition may block --share when cloud is not configured.
The documentation states --share should work "even if cloud is not configured", but line 556 requires cloudConfig.isEnabled() to be true. The createShareableModelAuditUrl function handles this case by falling back to getShareApiBaseUrl() when cloud is not enabled, but this code path prevents it from being called.
🔎 Proposed fix
- if (wantsToShare && cloudConfig.isEnabled()) {
+ if (wantsToShare) {
sharePromise = createShareableModelAuditUrl(audit);
}Alternatively, if cloud configuration is actually required for sharing to work, update the documentation to remove the claim that --share works without cloud configuration.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Determine if we should share | |
| const hasExplicitDisable = | |
| options.noShare === true || getEnvBool('PROMPTFOO_DISABLE_SHARING'); | |
| let wantsToShare: boolean; | |
| if (hasExplicitDisable) { | |
| wantsToShare = false; | |
| } else if (options.share === true) { | |
| wantsToShare = true; | |
| } else { | |
| // Default: auto-share when cloud is enabled | |
| wantsToShare = cloudConfig.isEnabled(); | |
| } | |
| logger.debug(`Model audit sharing decision: wantsToShare=${wantsToShare}`); | |
| // Start sharing in background (don't await yet - non-blocking like evals!) | |
| let sharePromise: Promise<string | null> | null = null; | |
| if (wantsToShare && cloudConfig.isEnabled()) { | |
| sharePromise = createShareableModelAuditUrl(audit); | |
| } | |
| // Determine if we should share | |
| const hasExplicitDisable = | |
| options.noShare === true || getEnvBool('PROMPTFOO_DISABLE_SHARING'); | |
| let wantsToShare: boolean; | |
| if (hasExplicitDisable) { | |
| wantsToShare = false; | |
| } else if (options.share === true) { | |
| wantsToShare = true; | |
| } else { | |
| // Default: auto-share when cloud is enabled | |
| wantsToShare = cloudConfig.isEnabled(); | |
| } | |
| logger.debug(`Model audit sharing decision: wantsToShare=${wantsToShare}`); | |
| // Start sharing in background (don't await yet - non-blocking like evals!) | |
| let sharePromise: Promise<string | null> | null = null; | |
| if (wantsToShare) { | |
| sharePromise = createShareableModelAuditUrl(audit); | |
| } |
🤖 Prompt for AI Agents
In @src/commands/modelScan.ts around lines 538-558, The sharing gate currently
prevents calling createShareableModelAuditUrl unless cloudConfig.isEnabled() is
true, which blocks explicit --share; change the runtime check so that once
wantsToShare is true (derived from options.share and hasExplicitDisable), you
call createShareableModelAuditUrl(audit) regardless of cloudConfig.isEnabled();
in other words, replace the conditional "if (wantsToShare &&
cloudConfig.isEnabled())" with "if (wantsToShare)" so
createShareableModelAuditUrl can perform its own fallback to
getShareApiBaseUrl().
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 which adds automatic sharing of model audit results to promptfoo Cloud. The changes introduce new CLI flags (--share and --no-share) and default to auto-sharing when cloud is enabled. I assessed this against all six LLM security vulnerability classes and found no LLM security vulnerabilities.
Minimum severity threshold for this scan: 🟡 Medium | Learn more
src/commands/modelScan.ts
Outdated
|
|
||
| // Display summary (unless JSON format requested by user) | ||
| // Determine if we should share | ||
| const hasExplicitDisable = options.noShare === true || getEnvBool('PROMPTFOO_DISABLE_SHARING'); |
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.
--no-share -> options.share == false. This should be updated to only check with options.share
- Fix --no-share flag by checking options.share === false (Commander.js sets share=false when --no-share is used, not noShare=true) - Use isModelAuditSharingEnabled() instead of cloudConfig.isEnabled() to match eval behavior and support custom share URLs - Remove misleading doc claim about --share working without cloud - Add comprehensive test coverage for all sharing scenarios 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Security Review ✅No critical issues found. This PR adds automatic sharing of model audit results to promptfoo Cloud with Security verification:
Implementation quality:
🟡 Minor Observations (1 item)
Test coverage:
PR conventions:
Last updated: 2026-01-07T23:35:00Z | Reviewing: 7f96a5c |
The note block duplicated information already implied by the non-blocking sharing implementation. The main documentation paragraph is sufficient. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…omptfoo into feat/model-audit-auto-share
Summary
Adds automatic sharing functionality to model audits with the same behavior as evaluation sharing. Model audit results are now automatically shared when cloud is enabled, providing a web-based interface for viewing, analyzing, and collaborating on scan results.
Motivation
Currently, model audit results can only be shared manually using
promptfoo share <scan-id>. This PR brings model audits to feature parity with evaluations by:--shareand--no-shareflagsPROMPTFOO_DISABLE_SHARINGenvironment variableChanges
Core Implementation
File:
src/commands/modelScan.ts--shareand--no-shareCLI flagsFile:
src/util/modelAuditCliParser.tsModelAuditCliOptionsSchemato includeshareandnoShareoptionsFile:
site/docs/model-audit/usage.mdBehavior
Sharing Decision Precedence (matches eval behavior):
PROMPTFOO_DISABLE_SHARING=true→ Never share (highest priority)--no-shareflag → Never share--shareflag → Always shareNon-Blocking Upload Flow:
Server-Side Deduplication (already implemented):
Usage Examples
Testing
--shareand--no-shareflagsDeveloper Experience Benefits
Breaking Changes
None. This is purely additive functionality with sensible defaults.