Skip to content

Running evaluation on mini-bc-agent - #11

Merged
Sun Haoran (haoranpb) merged 18 commits into
mainfrom
features/evaluate-mini
Oct 10, 2025
Merged

Running evaluation on mini-bc-agent#11
Sun Haoran (haoranpb) merged 18 commits into
mainfrom
features/evaluate-mini

Conversation

@haoranpb

@haoranpb Sun Haoran (haoranpb) commented Oct 9, 2025

Copy link
Copy Markdown
Collaborator

Potential fix for https://github.com/microsoft/BC-Bench/security/code-scanning/7

The logging of PowerShell commands should never include sensitive information such as passwords in cleartext.
To fix:

  • Before logging the command in _execute_powershell, scan for the relevant password assignment (ConvertTo-SecureString ... -AsPlainText -Force) and redact the password value from the string being logged.
  • This can be done safely by using a regular expression to replace assignments to $password (or other credentials) in the stringified command with a placeholder such as '******'.
  • The logging lines (lines 122 and 124), which show a preview and optionally the full command, should use the redacted string.
  • These changes are all within src/bcbench/agent/mini/bc_environment.py, within the _execute_powershell method.
  • Any required imports (such as for re) should be added at the top of the file if not already present.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

Sun Haoran (haoranpb) and others added 6 commits October 9, 2025 08:20
@haoranpb
fix github action matrix input
99200d9
Initial plan
c618eac
@haoranpb
Remove version field from schema and delete Evaluate.ps1
0147984 
Co-authored-by: haoranpb <27280733+haoranpb@users.noreply.github.com>
@haoranpb
Update README.md
2179d62
@haoranpb
Remove deprecated version field from schema and delete Evaluate.ps1 (#10
461d5e5 
)
@haoranpb @github-advanced-security
Potential fix for code scanning alert no. 7: Clear-text logging of se…
1cd8f3f 
…nsitive information

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@haoranpb
Sun Haoran (haoranpb) marked this pull request as ready for review October 9, 2025 06:50
@haoranpb Sun Haoran (haoranpb) changed the title Potential fix for code scanning alert no. 7: Clear-text logging of sensitive information Running evaluation on mini-bc-agent Oct 9, 2025
github-advanced-security[bot]
github-advanced-security AI found potential problems Oct 9, 2025
View reviewed changes
Comment thread src/bcbench/agent/mini/bc_environment.py Fixed
if len(command) > 100:
logger.debug(f"Full command: {command}")
if len(redacted_command) > 100:
logger.debug(f"Full command: {redacted_command}")

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information High

This expression logs sensitive data (password) as clear text.
This expression logs sensitive data (password) as clear text.
This expression logs sensitive data (password) as clear text.
This expression logs sensitive data (password) as clear text.
This expression logs sensitive data (password) as clear text.

Copilot Autofix

AI 11 months ago

To robustly prevent logging of sensitive command contents, especially those containing passwords, avoid logging any command that contains $password = ConvertTo-SecureString or similar secret-setting operations.
Specifically:

  • In _execute_powershell, enhance the logic so that anytime the command contains the pattern for setting $password, both INFO and DEBUG logging of the command or its preview/full content must be suppressed, regardless of the logging level or redaction.
  • Optionally, you can log a generic message, e.g., "Executing sensitive command (redacted)", but never the command itself.
  • The change should be made in src/bcbench/agent/mini/bc_environment.py in the block covering lines 103–119.
  • No additional imports are needed; use existing logging and re functionalities.
Suggested changeset 1
src/bcbench/agent/mini/bc_environment.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/bcbench/agent/mini/bc_environment.py b/src/bcbench/agent/mini/bc_environment.py
--- a/src/bcbench/agent/mini/bc_environment.py
+++ b/src/bcbench/agent/mini/bc_environment.py
@@ -111,12 +111,15 @@
                     cmd,
                 )
 
-            redacted_command = redact_passwords(command)
-            command_preview: str = redacted_command if len(redacted_command) <= 100 else redacted_command[:97] + "..."
-            logger.info(f"Executing:\n{command_preview}")
-            if len(redacted_command) > 100:
-                logger.debug(f"Full command: {redacted_command}")
-
+            if re.search(r"\$password\s*=\s*ConvertTo-SecureString", command):
+                logger.info("Executing sensitive PowerShell command (command redacted)")
+                logger.debug("Sensitive PowerShell command detected. Full command redacted from logs.")
+            else:
+                redacted_command = redact_passwords(command)
+                command_preview: str = redacted_command if len(redacted_command) <= 100 else redacted_command[:97] + "..."
+                logger.info(f"Executing:\n{command_preview}")
+                if len(redacted_command) > 100:
+                    logger.debug(f"Full command: {redacted_command}")
         try:
             result = subprocess.run(
                 ["pwsh", "-NoProfile", "-NonInteractive", "-Command", command], cwd=working_dir, capture_output=True, text=True, timeout=timeout, env={**os.environ, **self.config.env}
EOF
@@ -111,12 +111,15 @@
cmd,
)

redacted_command = redact_passwords(command)
command_preview: str = redacted_command if len(redacted_command) <= 100 else redacted_command[:97] + "..."
logger.info(f"Executing:\n{command_preview}")
if len(redacted_command) > 100:
logger.debug(f"Full command: {redacted_command}")

if re.search(r"\$password\s*=\s*ConvertTo-SecureString", command):
logger.info("Executing sensitive PowerShell command (command redacted)")
logger.debug("Sensitive PowerShell command detected. Full command redacted from logs.")
else:
redacted_command = redact_passwords(command)
command_preview: str = redacted_command if len(redacted_command) <= 100 else redacted_command[:97] + "..."
logger.info(f"Executing:\n{command_preview}")
if len(redacted_command) > 100:
logger.debug(f"Full command: {redacted_command}")
try:
result = subprocess.run(
["pwsh", "-NoProfile", "-NonInteractive", "-Command", command], cwd=working_dir, capture_output=True, text=True, timeout=timeout, env={**os.environ, **self.config.env}
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated

@haoranpb Sun Haoran (haoranpb) Oct 9, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Can you see the handling I have done in the logger util?

@haoranpb
Sun Haoran (haoranpb) merged commit 6dd6665 into main Oct 10, 2025
4 checks passed
@haoranpb
Sun Haoran (haoranpb) deleted the features/evaluate-mini branch October 10, 2025 08:54
Marko Aleksandric (AleksandricMarko) added a commit that referenced this pull request Jul 31, 2026
@AleksandricMarko
Address PR #762 review: rename ext categories, LMChecklist triage gra…
9ef01ab 
…ding

Applies Haoran's review feedback for the extensibility-request categories:

- Rename to explicit `extensibility-request-implement` / `extensibility-request-triage`
  across category ids, enum members, classes, dataset files and module files (#1/#2/#4).
- Slim both prompt templates to reflect real-world usage; category-specific rules now
  live in the ai-ext-fix / argus-triage skills (#3).
- Merge duplicate `bcbench.results` import in commands/evaluate.py (#5).
- Uptake #761: both ext entries subclass `RepoGroundedEntry` (#6/#7).
- Triage `patch` defaults to None; stripped from the dataset (#8).
- Constrain triage label fields with a `ManagedLabel` Literal to catch dataset typos (#9).
- Move the ext dataset classes into dataset/extensibility_request.py, like code review (#10).
- Drop `fetch_commit_if_missing` from both ext pipelines to match the other pipelines (#11).
- Keep ext-implement judge-only for now; container build/publish noted as future work (#12).
- Grade triage with the NL2AL LMChecklist instead of the code-review-style hybrid:
  `ExtRequestTriageResult` removed in favour of `JudgeBasedEvaluationResult`, expected_*
  fields replaced by an `expected` checklist, evaluators `lm_checklist`, core score
  `test_passed`; 17 triage entries migrated (#13/#14/#15/#16).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: fb712366-b026-470c-9d09-1bd9e9f52a31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@haoranpb @github-advanced-security