mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
eval-exec-mai
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Viktor Hofer
|
d97128e5d9 |
Move static analysis checks from eval to check command (#425)
* Move static analysis checks from eval to check command Remove SkillProfiler.AnalyzeSkill from the evaluate command's per-skill flow. All spec conformance checks (name, description, compatibility, body line count, file reference depth, token size warnings, etc.) are already enforced by the check command — running them again in eval was redundant. The eval-specific 'skill name in eval prompt' check is extracted into EvaluateCommand.ValidateEvalPrompts and kept in eval since it requires eval config context. The evalConfig parameter is removed from SkillProfiler.AnalyzeSkill along with the dead ResourceFileCount field. Tests updated to exercise ValidateEvalPrompts directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * [WIP] [WIP] Address feedback: Remove ProfileWarnings from vedict model (#426) * Initial plan * Remove ProfileWarnings from verdict model and reporter output Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/skills/sessions/9b7c3d0a-1693-4238-a397-2200f83634b5 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ViktorHofer <7412651+ViktorHofer@users.noreply.github.com> --------- Co-authored-by: Viktor Hofer <vihofer@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> |
||
|
Abhitej John
|
bb974f7360 |
Security Update (#331)
* Security Update Identified and remediated 15 vulnerabilities across the skill-validator runtime, dashboard, and CI/CD pipelines. Changes: - Runtime (AgentRunner.cs): path traversal prevention, deny-by-default permissions, env scrubbing, MCP server hardening - Regex (AssertionEvaluator.cs): ReDoS timeout - Logging (SkillDiscovery.cs): exception logging - Dashboard (dashboard.html, dashboard.js): SRI hash, XSS escaping - CI/CD (5 workflow files): SHA pinning, persist-credentials: false - Tests (RunnerTests.cs): 45+ new security test cases MCP Server Defense-in-Depth (5 layers): 1. Command allowlist: only dotnet, node, npx, python, python3, uvx 2. Path rejection: no /, \, or .. in command names 3. Args validation: per-runtime dangerous flag blocklist 4. Env sanitization: strips PATH, LD_PRELOAD, NODE_OPTIONS, DOTNET_STARTUP_HOOKS, etc. 5. Cwd dropped: custom working directories not passed to SDK Subprocess Isolation: - Sensitive env vars + prefix patterns scrubbed from child processes - 120-second timeout with Kill(entireProcessTree: true) - Deny-by-default permission model Supply Chain: - All GitHub Action references pinned to full commit SHAs - All checkout steps set persist-credentials: false - Chart.js CDN pinned to v4.4.7 with SRI integrity hash * Update .github/workflows/skill-validator.yml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update eng/skill-validator/src/Services/AgentRunner.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update eng/skill-validator/src/Services/AgentRunner.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update tests to use fullCommandText field name Update ExtractsCommandProperty and PrefersPathOverCommand tests to use the fullCommandText property instead of command, matching the updated permission request schema in AgentRunner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add 'dnx' to allowed MCP commands * Address PR review feedback: security hardening fixes - CheckPermission: normalize paths with TrimEndingDirectorySeparator to prevent double-separator false denials when dirs have trailing slash - SanitizeMcpArgs: reject prefix forms (-econsole.log) and --flag=value variants that bypass exact-match blocklist checks - Setup file path traversal: normalize canonicalWorkDir and canonicalSkillPath to avoid double-separator in StartsWith checks - IsAllowedMcpCommand: use GetFileName instead of GetFileNameWithoutExtension to block dotnet.bat/node.cmd; only strip .exe on Windows Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * prune * fix * Fix concurrency setting * Add binlog generation and upload to skill-validator workflow Produce binary logs for build, test, and pack steps using /bl: flag. Upload binlogs as artifacts (with if: always()) for build diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve CheckPermission diagnostics and fix AOT build error Log allowed directories alongside denied paths to help diagnose permission mismatches. Replace JsonSerializer.Serialize with manual string building to fix IL3050 AOT compilation error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Extract fileName from permission requests Some permission requests use 'fileName' instead of 'path'. Handle this property to avoid false denials for file-level operations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * nownar nu1903 temporarily * Log config dir in verbose mode alongside work dir Include skilled/baseline label for consistency with the work dir log. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add skilled/baseline label to permission denial logs Helps identify which run type triggered the denial when debugging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix empty runLabel brackets and add labels to judge permission checks Conditionally format the label suffix to avoid empty () when no runLabel is supplied. Pass 'judge' and 'pairwise-judge' labels from the respective judge call sites. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Allow pairwise judge to read from skilled work directory The pairwise judge compares baseline and skilled runs in one session, but previously only allowed reads from the baseline work directory. Add additionalAllowedDirs parameter to CheckPermission and pass the skilled work directory through PairwiseJudgeOptions.SkilledWorkDir. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix tests to match allow-by-default for unrecognized permission requests CheckPermission was changed to allow requests with no extractable path/command (allow-by-default), but four tests still expected the old deny-by-default behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove NoWarn * Pin discover job checkout action to full commit SHA Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Dan Moseley <danmose@microsoft.com> |
||
|
Jan Krivanek
|
e1f513f486 |
[Infra] Improve judges retry and resiliency (#188)
* Add better retry for judge * Extract funtionality and guard against single scenario failing whole eval run * Reflect PR comments * Reflect PR comments * Apply timeout and parallelism defaults to evaluation-run.yml Upstream refactored the evaluation workflow to use a reusable workflow (evaluation-run.yml). Move our timeout (60->90min) and parallelism (3->5) changes to that file. |
||
|
Viktor Hofer
|
cfc22971e1 |
Port skill-validator from TypeScript to .NET C# (#106)
* Port skill-validator from TypeScript to .NET C# Replace the TypeScript skill-validator with a .NET 10 C# implementation. Uses System.CommandLine for CLI parsing, GitHub.Copilot.SDK for Copilot communication, YamlDotNet for YAML, and xUnit v3 with MTP v2 for testing. All 135 unit tests ported and passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update CI workflows for .NET skill-validator Replace Node.js setup and npm commands with dotnet build/test/publish. Use global.json for SDK version instead of hardcoded dotnet-version. Publish self-contained binary for evaluate runners. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix permission denied for Copilot SDK native binary in CI Restore execute permissions on the native copilot binary after artifact download, which strips file permissions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * nit updates * Apply upstream changes: --verdict-warn-only, safeDirName, fix benches Port three origin/main commits to the C# skill-validator and workflow: - Add --verdict-warn-only flag replacing --strict; verdict failures (threshold, completion, activation) become warnings while execution errors and --require-evals missing_eval failures remain fatal. - Add FailureKind tracking to SkillVerdict and Comparator. - Add SafeDirName sanitizer for skill directory names in Reporter. - Add per-skill verdict.json generation in JSON reporter. - Update evaluation.yml: remove continue-on-error, use --verdict-warn-only, change default infra runs to 2, skip comment-on-pr on cancellation, use ResultsFile for benchmark data generation. - Update generate-benchmark-data.ps1 to read verdicts from results.json. - Update README with new CLI flags and CI example. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update CONTRIBUTING.md for .NET skill-validator Replace Node.js/npm prerequisites and commands with .NET 10 SDK equivalents (dotnet run). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update test commands in CONTRIBUTING.md to use .csproj file * Add RunArguments for default results-dir and verbose logging - Set RunArguments in SkillValidator.csproj using MSBuild NormalizeDirectory to pass --results-dir pointing to artifacts/TestResults/skill-validator/ - Log configured results dir at startup when --verbose is enabled Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix path * Fix AgentRunner: capture tool completion events and implement permission checking - Add ToolExecutionCompleteEvent handler to capture Success/Result data from tool completions, fixing timeline showing all tools as FAIL - Add ToolExecutionStartEvent arguments capture for timeline detail - Implement CheckPermission with path extraction from PermissionRequest ExtensionData, replacing the stub that always returned true - Update Judge and PairwiseJudge to pass PermissionRequest directly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix CheckPermission tests for cross-platform CI (Linux) Use Path.GetTempPath() and Path.Combine() instead of hardcoded Windows paths so tests pass on both Windows and Linux CI runners. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Capture skills & tokens usages * Fix thread safety, resource leaks, and null safety in skill-validator - Use ConcurrentBag and SemaphoreSlim with double-check locking for _workDirs and _sharedClient in AgentRunner (race conditions under parallel runs/scenarios) - Make Spinner IDisposable; protect _frame/_message/_active with lock and Interlocked; use 'using var' at call site - Use 'using var' for timeout Timers in Judge and PairwiseJudge to prevent leaks on exception paths - Replace perRunPairwise[0] fallback with FirstOrDefault() to avoid IndexOutOfRangeException when list is empty Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update eng/skill-validator/README.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Address PR review feedback - Add per-type required field validation in ParseAssertion (e.g. file_contains requires both path and value, output_matches requires pattern) - Fix JsonDocument leaks in LlmJson: use 'using' and Clone() on RootElement in both ParseLlmJson and TryParseJson - Pin Microsoft.Extensions.FileSystemGlobbing to 10.0.0 instead of floating 10.* for reproducible builds - Fix README Development section to reference SkillValidator.slnx instead of .sln Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jan Krivanek <jankrivanek@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> |