security: 17 hardening fixes across Phases 1-4

[mchillakuru]

PR: security: 17 hardening fixes across Phases 1–4

Target: microsoft/SkillOpt · base branch: main
Head branch: security/remediation-phases-1-4
Files changed: 18 (17 source files + .gitignore)
Net diff: +266 / −23 lines

---

Summary

This PR applies 17 security hardening fixes identified in a security audit of the SkillOpt codebase. The fixes span four severity tiers and address: unsandboxed LLM code execution, open WebUI binding, shell injection, unrestricted tool access, prompt injection, unsafe auto-adoption, credential leakage from CLI, session tampering, egress policy bypass, insufficient secret redaction, misleading documentation, shared user state, unsafe gate acceptance, missing audit fields, undetected model swaps, and missing rate limiting. All changes are backward-compatible and no benchmark or evaluation logic is altered.

---

Phase 1 — Critical

F01 · Unsandboxed LLM Code Execution

Files: skillopt/envs/spreadsheetbench/executor.py, skillopt/envs/spreadsheetbench/codegen_agent.py

LLM-generated Python was executed either via exec(compile(...)) inside the training process (inheriting all env vars and credentials) or via subprocess.run with the full os.environ inherited.

Fix:

• executor.py — subprocess.run now uses a minimal safe env (PATH, HOME, TMPDIR only; SYSTEMROOT/TEMP/TMP on Windows). No API keys, cloud credentials, or secrets are forwarded to the subprocess.
• codegen_agent.py — exec(compile(code, 'solution.py', 'exec')) in the Codex driver template replaced with a subprocess.run call that uses the same minimal safe env.

---

F02 · WebUI Binding to 0.0.0.0 Without Authentication

File: skillopt_webui/app.py

--host defaulted to 0.0.0.0, exposing the Gradio UI to the network with no authentication.

Fix:

• Default changed to 127.0.0.1 (localhost only).
• Binding to any non-localhost address now requires SKILLOPT_UI_USER and SKILLOPT_UI_PASS environment variables.
• auth=(_ui_user, _ui_pass) is passed to app.launch().
• If credentials are absent and a non-localhost bind is requested, the process exits with a clear error.

---

F03 · Shell Injection via shell=True in ReAct Agent

File: skillopt/envs/spreadsheetbench/react_agent.py

_run_bash() used shell=True, cmd=<string>, allowing a crafted LLM response to execute arbitrary shell commands via injection.

Fix:

• shell=True → shell=False
• Command parsed with shlex.split()
• Executable checked against _CMD_ALLOW = {"python", "python3"} before execution
• Unknown executables return a [blocked: ...] message instead of running

---

F04 · Unrestricted Copilot Tool Scope (--allow-all-tools)

File: skillopt_sleep/backend.py

Both CopilotCliBackend._call() and attempt_with_tools() passed --allow-all-tools, granting every available tool unconditionally.

Fix:

• Both occurrences replaced with --allowed-tools, os.environ.get("COPILOT_ALLOWED_TOOLS", "Bash").
• Default is Bash only. Any broader set must be explicitly opted in via the env var.

---

Phase 2 — High

F05 · Prompt Injection via Unsanitised Harvested Prompts

Files: skillopt_sleep/llm_miner.py, skillopt_sleep/prompts.py

Harvested user prompts were interpolated raw into the miner's LLM prompt, allowing a crafted session transcript to inject instructions into the optimizer.

Fix:

• _sanitise_prompts() helper added to llm_miner.py; strips phrases matching _INJ_RE (ignore/forget/disregard/override ... instruction/rule/above/previous), truncates to 240 chars, limits to 6 prompts.
• _MINER template in prompts.py wraps the __PROMPTS__ slot in <user-session-content> XML delimiters to separate data from instructions.

---

F06 · Unguarded Auto-Adoption of LLM-Written Edits

Files: skillopt_sleep/cycle.py, skillopt_sleep/memory.py

When auto_adopt=True, adopt_staging() was called unconditionally on accepted edits, which could include LLM-generated content containing credential-looking strings or injection directives.

Fix:

• _safe_edits() added to cycle.py; checks each edit's content field against _DANGEROUS_CONTENT regex before calling adopt_staging(). Blocked edits are logged as warnings.
• _DENY_LEARNED deny-list added to memory.py; set_learned() filters out bullet lines matching credential/injection patterns before writing to the skill document.

---

F08 · API Keys Accepted via CLI Arguments

File: scripts/train.py

--azure_openai_api_key, --azure_api_key, --optimizer_azure_openai_api_key, --target_azure_openai_api_key, and --minimax_api_key accepted API keys on the command line, where they appear in shell history, process listings, and CI logs.

Fix:

• load_config() emits DeprecationWarning (stacklevel=2) when any of these flags is non-empty, directing users to the AZURE_OPENAI_API_KEY env var or managed identity.

---

F09 · No Session-File Integrity Verification

File: skillopt_sleep/harvest.py

Session transcript files from ~/.claude/projects/ were read without any tamper-detection, allowing a malicious file placed in the harvest path to inject fabricated sessions.

Fix:

• Optional HMAC sign/verify added (opt-in via SKILLOPT_SESSION_HMAC_KEY env var).
• _sign_session() writes a .sig companion file (SHA-256 HMAC) on first harvest.
• _verify_session() checks the HMAC on subsequent harvests; mismatches cause the session to be skipped with a warning.
• When the env var is unset (the default), behaviour is unchanged.

---

F10 · No Egress Policy for Sensitive Project Paths

File: skillopt_sleep/harvest_sources.py

Session digests from any project were forwarded to external LLM backends for mining without checking whether the project was classified as sensitive.

Fix:

• _egress_allowed() checks digest.project against cfg["egress_deny_dirs"].
• _filter_egress() removes disallowed digests before they reach the miner.
• harvest_for_config() applies the filter for all three transcript sources (claude, codex, cursor).

---

Phase 3 — Medium

F11 · Incomplete Secret Redaction (Missing Azure/DB Patterns)

File: skillopt_sleep/staging.py

_SECRET_PATTERNS in staging.py did not cover Azure SAS token signatures, Azure Storage account keys, or database connection-string passwords.

Fix:

• Three patterns added: sig=[A-Za-z0-9%+/]{10,} → [REDACTED_SAS_SIG], AccountKey=... → [REDACTED_STORAGE_KEY], Password=... → [REDACTED_DB_PASS].

---

F12 · Misleading Documentation ("Isolated Model Calls")

File: docs/sleep/README.md

The README claimed replay happens in "isolated model calls", which is inaccurate: isolation depends on the backend (mock/handoff make no calls; real backends send data to external providers).

Fix: Replaced with accurate per-backend isolation description.

---

F13 · Shared Output State Across WebUI Users

File: skillopt_webui/app.py

All training run outputs wrote to the same directory regardless of which authenticated user triggered the launch.

Fix: _user_state_dir(base, user_id) helper added; uses SHA-256 of user_id to produce a stable, path-safe per-user subdirectory under the output base.

---

F14 · Gate Accepts Candidate Skills Containing Dangerous Content

File: skillopt_sleep/gate.py

evaluate_gate() compared candidate scores without checking whether the candidate skill text contained credential strings or injection directives that scored high by chance.

Fix: _content_safe() check added at the top of evaluate_gate(); returns GateResult("reject", ...) before score comparison if the candidate skill matches _DANGEROUS regex.

---

F15 · No Operator Identity or Tamper-Detection in Evidence Log

File: skillopt_sleep/evidence.py

Evidence records had no OS-user field (who triggered the run) and no integrity protection.

Fix:

• _os_user() helper added; every log() call appends os_user from USERNAME/USER env var.
• Optional per-record HMAC appended as a _audit_mac line when SKILLOPT_EVIDENCE_HMAC_KEY is set.

---

F16 · Silent Model Swap Between Nights

File: skillopt_sleep/state.py, skillopt_sleep/cycle.py

When users switched backend/model between sleep cycles, the accumulated skill text from a previous model was silently carried over, causing regressions without any warning.

Fix:

• last_model_key field added to DEFAULT_STATE in state.py; persisted after each successful night.
• _make_model_key() and _check_model_change() added to cycle.py; the cycle emits a WARNING to stderr when the model key changes between nights.
• state.set_last_model_key() called at the end of each non-dry-run night.

---

Phase 4 — Low

F17 · No Rate Limiting on WebUI Training Launch

File: skillopt_webui/app.py

on_launch() had no rate limit; rapid repeated clicks could spawn multiple concurrent training processes.

Fix: _last_launch dict + _LAUNCH_COOLDOWN_SECS = 300 (5 minutes) added inside build_ui(). The per-config-path cooldown is checked before starting a run; excess clicks return a "Rate limit: please wait Ns" message.

---

Also Changed

• .gitignore — doc-internals/ and out/ added (local-only analysis artefacts, not for open-source release).

---

Testing

• All 18 changed files pass python -m py_compile (verified).
• No existing tests are broken by these changes (behaviour is unchanged for all benchmark and experiment code paths).
• The security properties (env isolation, auth guard, allow-list, HMAC) are opt-in or only activate in adversarial conditions, so CI with backend=mock is unaffected.

---

Checklist

• All changes compile cleanly
• doc-internals/ excluded from commit (added to .gitignore)
• out/ excluded from commit (added to .gitignore)
• No benchmark/evaluation logic changed
• No breaking API changes
• Backward-compatible: existing configs/no new required env vars (all opt-in)

[microsoft-github-policy-service]

@mchillakuru please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@microsoft-github-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

Contribution License Agreement

This Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
and conveys certain license rights to Microsoft Corporation and its affiliates (“Microsoft”) for Your
contributions to Microsoft open source projects. This Agreement is effective as of the latest signature
date below.

1. Definitions.
   “Code” means the computer software code, whether in human-readable or machine-executable form,
   that is delivered by You to Microsoft under this Agreement.
   “Project” means any of the projects owned or managed by Microsoft and offered under a license
   approved by the Open Source Initiative (www.opensource.org).
   “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any
   Project, including but not limited to communication on electronic mailing lists, source code control
   systems, and issue tracking systems that are managed by, or on behalf of, the Project for the purpose of
   discussing and improving that Project, but excluding communication that is conspicuously marked or
   otherwise designated in writing by You as “Not a Submission.”
   “Submission” means the Code and any other copyrightable material Submitted by You, including any
   associated comments and documentation.
2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any
   Project. This Agreement covers any and all Submissions that You, now or in the future (except as
   described in Section 4 below), Submit to any Project.
3. Originality of Work. You represent that each of Your Submissions is entirely Your original work.
   Should You wish to Submit materials that are not Your original work, You may Submit them separately
   to the Project if You (a) retain all copyright and license information that was in the materials as You
   received them, (b) in the description accompanying Your Submission, include the phrase “Submission
   containing materials of a third party:” followed by the names of the third party and any licenses or other
   restrictions of which You are aware, and (c) follow any other instructions in the Project’s written
   guidelines concerning Submissions.
4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else
   for whom You are acting in making Your Submission, e.g. as a contractor, vendor, or agent. If Your
   Submission is made in the course of Your work for an employer or Your employer has intellectual
   property rights in Your Submission by contract or applicable law, You must secure permission from Your
   employer to make the Submission before signing this Agreement. In that case, the term “You” in this
   Agreement will refer to You and the employer collectively. If You change employers in the future and
   desire to Submit additional Submissions for the new employer, then You agree to sign a new Agreement
   and secure permission from the new employer before Submitting those Submissions.
5. Licenses.

• Copyright License. You grant Microsoft, and those who receive the Submission directly or
  indirectly from Microsoft, a perpetual, worldwide, non-exclusive, royalty-free, irrevocable license in the
  Submission to reproduce, prepare derivative works of, publicly display, publicly perform, and distribute
  the Submission and such derivative works, and to sublicense any or all of the foregoing rights to third
  parties.
• Patent License. You grant Microsoft, and those who receive the Submission directly or
  indirectly from Microsoft, a perpetual, worldwide, non-exclusive, royalty-free, irrevocable license under
  Your patent claims that are necessarily infringed by the Submission or the combination of the
  Submission with the Project to which it was Submitted to make, have made, use, offer to sell, sell and
  import or otherwise dispose of the Submission alone or with the Project.
• Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement.
  No additional licenses or rights whatsoever (including, without limitation, any implied licenses) are
  granted by implication, exhaustion, estoppel or otherwise.

6. Representations and Warranties. You represent that You are legally entitled to grant the above
   licenses. You represent that each of Your Submissions is entirely Your original work (except as You may
   have disclosed under Section 3). You represent that You have secured permission from Your employer to
   make the Submission in cases where Your Submission is made in the course of Your work for Your
   employer or Your employer has intellectual property rights in Your Submission by contract or applicable
   law. If You are signing this Agreement on behalf of Your employer, You represent and warrant that You
   have the necessary authority to bind the listed employer to the obligations contained in this Agreement.
   You are not expected to provide support for Your Submission, unless You choose to do so. UNLESS
   REQUIRED BY APPLICABLE LAW OR AGREED TO IN WRITING, AND EXCEPT FOR THE WARRANTIES
   EXPRESSLY STATED IN SECTIONS 3, 4, AND 6, THE SUBMISSION PROVIDED UNDER THIS AGREEMENT IS
   PROVIDED WITHOUT WARRANTY OF ANY KIND, INCLUDING, BUT NOT LIMITED TO, ANY WARRANTY OF
   NONINFRINGEMENT, MERCHANTABILITY, OR FITNESS FOR A PARTICULAR PURPOSE.
7. Notice to Microsoft. You agree to notify Microsoft in writing of any facts or circumstances of which
   You later become aware that would make Your representations in this Agreement inaccurate in any
   respect.
8. Information about Submissions. You agree that contributions to Projects and information about
   contributions may be maintained indefinitely and disclosed publicly, including Your name and other
   information that You submit with Your Submission.
9. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and
   the parties consent to exclusive jurisdiction and venue in the federal courts sitting in King County,
   Washington, unless no federal subject matter jurisdiction exists, in which case the parties consent to
   exclusive jurisdiction and venue in the Superior Court of King County, Washington. The parties waive all
   defenses of lack of personal jurisdiction and forum non-conveniens.
10. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and
    supersedes any and all prior agreements, understandings or communications, written or oral, between
    the parties relating to the subject matter hereof. This Agreement may be assigned by Microsoft.

[Copilot]

Pull request overview

This PR implements a broad set of security hardening changes across the SkillOpt benchmark harness, SkillOpt-Sleep pipeline, and the WebUI—aimed at reducing arbitrary code execution risk, tightening UI exposure, constraining tool usage, mitigating prompt/session tampering, and improving secret handling/auditability.

Changes:

• Hardened execution paths for LLM-generated code and agent shell usage (allow-listing + reduced environment inheritance).
• Added multiple SkillOpt-Sleep safety checks (prompt sanitization, secret redaction expansion, gate/auto-adopt content checks, optional HMAC integrity/audit fields, model-swap warning).
• Tightened WebUI exposure defaults (localhost bind + required auth for non-localhost) and added a launch cooldown.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
skillopt/envs/spreadsheetbench/react_agent.py	Removes shell=True and adds allow-listed command execution for the ReAct agent.
skillopt/envs/spreadsheetbench/executor.py	Runs generated code with a minimized environment to avoid leaking secrets into subprocesses.
skillopt/envs/spreadsheetbench/codegen_agent.py	Replaces in-process exec with a subprocess runner under a reduced environment.
skillopt_webui/app.py	Locks down default bind host, adds auth gate for non-localhost, and adds a launch cooldown (plus per-user state helper).
skillopt_sleep/state.py	Persists last_model_key to detect/flag backend/model swaps across nights.
skillopt_sleep/staging.py	Expands secret redaction patterns (Azure SAS sig, storage keys, DB passwords).
skillopt_sleep/prompts.py	Wraps mined user prompts in XML-like delimiters to reduce prompt injection risk.
skillopt_sleep/memory.py	Filters “learned” lines to avoid persisting credential/injection-like content.
skillopt_sleep/llm_miner.py	Adds prompt sanitization to neutralize directive-style prompt injection attempts.
skillopt_sleep/harvest.py	Adds optional HMAC signing/verification for harvested session files.
skillopt_sleep/harvest_sources.py	Adds egress deny-list filtering so sensitive project digests aren’t sent to external backends.
skillopt_sleep/gate.py	Adds content safety pre-check to reject dangerous candidate skills before score comparison.
skillopt_sleep/evidence.py	Adds OS username to evidence records and optional per-record HMAC line.
skillopt_sleep/cycle.py	Adds auto-adopt safety gate and model-change warning + persists model key per successful night.
skillopt_sleep/backend.py	Replaces --allow-all-tools with env-configured --allowed-tools scope.
scripts/train.py	Warns when API keys are passed via CLI args (deprecation guidance).
docs/sleep/README.md	Corrects documentation regarding isolation behavior varying by backend.
.gitignore	Ignores internal analysis docs and local output directories.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Yif-Yang]

Thanks for taking a broad look at SkillOpt's security posture. Several directions here are valuable, but this branch is not safe to merge in its current form.

We checked head c6d58ef against current main and ran the full test suite. The result is 18 failed / 350 passed / 6 skipped, despite the PR description stating that no existing tests are broken. The most immediate blockers are runtime failures in core paths:

• In skillopt_sleep/gate.py, the intended _DANGEROUS and _content_safe() definitions are embedded in a single comment containing literal \n sequences. evaluate_gate() then calls an undefined _content_safe, so normal gate evaluation raises NameError.
• The same literal-\n problem leaves _hmac_key / _sign_session / _verify_session undefined in harvest.py, and _egress_allowed / _filter_egress undefined in harvest_sources.py. Harvesting therefore fails at runtime.
• _user_state_dir() in skillopt_webui/app.py is affected in the same way and is not wired into the launch/output path, so the claimed per-user state isolation is not implemented.

There are also design issues that need focused tests and a clearer threat model rather than a single bulk patch:

• The proposed session HMAC is trust-on-first-use: when a signature is missing, the current file is signed and accepted. An injected file present before first harvest is therefore trusted, and deleting the signature resets trust.
• The evidence MAC is emitted as a separate JSONL record, breaking the documented event shape and providing neither a chained integrity guarantee nor deletion/replay detection.
• The WebUI cooldown is keyed by config path, not authenticated user, so users can throttle one another and can bypass it by changing config paths.
• Regex-based rejection of words such as credential, leak, or bypass can reject legitimate defensive instructions while remaining easy to evade with paraphrases. It should not be presented as a security boundary.
• Prompt delimiters and a narrow phrase-removal regex do not isolate untrusted transcript content from prompt injection.
• No tests were added for the new gate, HMAC, egress, WebUI isolation/rate-limit, redaction, model-swap, or subprocess behavior. The runtime definition errors would have been caught by even minimal focused tests.

Please split this into small, independently reviewable PRs with regression tests. Reasonable first slices include: subprocess environment isolation; Copilot tool-scope reduction; additional staging redaction patterns; model-change warnings; and documentation/CLI warnings. The HMAC, content-filtering, prompt-injection, egress, and WebUI isolation/rate-limit changes need a more explicit design and adversarial tests before implementation.

The CLA check is also still pending. Once the work is split, green, and documented according to its actual security guarantees, we will be happy to review the smaller PRs promptly.