fix: security and correctness fixes across five scripts#118
Merged
Conversation
- setup_api.sh: Flask API now listens on 127.0.0.1 instead of 0.0.0.0 to avoid unintentional network exposure. - stealth_post.sh: GPG passphrase passed via --passphrase-fd to avoid process-list exposure; FTP credentials moved to a temp netrc file (chmod 600) instead of the curl --user flag; netrc file shred on exit. - pentest_verification.sh: Removed default 'admin'/'admin' GVM credentials (GVM_USER/GVM_PASSWORD are now required); added hostname character validation before interpolating into the XML scan request. - pentest_exploitation.sh: Fixed dry-run side-effect (output file was truncated even in dry-run mode); added missing argument validation for --results and --search-file options. - SharePointManagement.ps1: Fixed SPSite scope bug ($script:SPSite in Add-User); added -ErrorAction Stop to connection calls; added SiteUrl and Template/UserLogin validation in the action switch. - ExchangeOnlineManagement.ps1: Changed return to exit 1 for module-check failures (sets correct non-zero exit code); added try/catch to the 'list' and 'disconnect' actions.
- pentest_verification.sh: restrict hostname regex to [A-Za-z0-9._-] (remove colon which could enable XML namespace injection) - stealth_post.sh: replace herestring (<<<) with printf pipe for GPG passphrase to avoid potential temp-file creation - SharePointManagement.ps1: add SiteUrl validation for OnPrem mode in Connect-SP, consistent with Online mode validation
Copilot
AI
changed the title
[WIP] Fix critical issues from code review for safety and correctness
fix: security and correctness fixes across five scripts
Jul 1, 2026
There was a problem hiding this comment.
Pull request overview
This PR tightens security and correctness in several operational scripts (Bash and PowerShell), focusing on reducing credential exposure, preventing unsafe defaults/injection vectors, and making failure modes more explicit.
Changes:
- Restricts network exposure and improves authentication/credential handling (localhost-only Flask bind; GPG/curl credential hardening; OpenVAS credential defaults removed).
- Improves runtime safety and correctness (dry-run avoids side effects; missing argument validation; PowerShell scope fixes; stronger error handling/exit behavior).
- Adds input validation to reduce injection risk (hostname allowlist before interpolating into OpenVAS XML).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/powershell/SharePointManagement.ps1 | Adds stricter SiteUrl validation, fixes On-Prem $SPSite scoping, and validates required parameters per action. |
| scripts/powershell/ExchangeOnlineManagement.ps1 | Fixes module-missing exit code and adds try/catch around list/disconnect actions. |
| scripts/linux/stealth_post.sh | Avoids exposing GPG/FTP secrets in process list by switching to --passphrase-fd and a temp netrc file. |
| scripts/linux/setup_api.sh | Binds the Flask app to localhost instead of all interfaces. |
| scripts/linux/pentest_verification.sh | Removes default OpenVAS creds and adds hostname allowlist before embedding into XML. |
| scripts/linux/pentest_exploitation.sh | Adds missing CLI value validation and prevents dry-run from truncating output files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
95
to
+99
| OUT="$(mktemp)" | ||
| ENC_OUT="$OUT.gpg" | ||
| trap 'rm -f "$OUT" "$ENC_OUT"' EXIT | ||
| NETRC_FILE="$(mktemp)" | ||
| trap 'rm -f "$OUT" "$ENC_OUT" "$NETRC_FILE"' EXIT | ||
| chmod 600 "$NETRC_FILE" |
Comment on lines
+110
to
112
| # Pass passphrase via pipe to avoid exposure in the process list. | ||
| if ! printf '%s' "$GPG_PASSPHRASE" | gpg --batch --yes --passphrase-fd 0 -c "$OUT"; then | ||
| echo "gpg encryption failed." >&2 |
Comment on lines
+277
to
+282
| elif [[ -z "${GVM_USER:-}" || -z "${GVM_PASSWORD:-}" ]]; then | ||
| echo "GVM_USER and GVM_PASSWORD must be set to run OpenVAS scans; skipping." >&2 | ||
| record "$host" "openvas" "skipped" "credentials not configured" | ||
| elif [[ ! "$host" =~ ^[A-Za-z0-9._-]+$ ]]; then | ||
| echo "Host '$host' contains characters unsafe for XML; skipping OpenVAS." >&2 | ||
| record "$host" "openvas" "skipped" "unsafe hostname" |
Comment on lines
73
to
+78
| 'list' { | ||
| Get-Mailbox | ||
| try { | ||
| Get-Mailbox -ErrorAction Stop | ||
| } catch { | ||
| Write-Error "Failed to list mailboxes. $_" | ||
| } |
Comment on lines
160
to
+165
| 'disconnect' { | ||
| Disconnect-ExchangeOnline -Confirm:$false | ||
| try { | ||
| Disconnect-ExchangeOnline -Confirm:$false -ErrorAction Stop | ||
| } catch { | ||
| Write-Error "Failed to disconnect. $_" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Targeted fixes for the highest-priority correctness and security issues surfaced in the code review: credential exposure in process lists, unauthenticated network-exposed API, hardcoded default credentials, XML injection risk, dry-run side-effects, and PowerShell scope/exit-code bugs.
Changes
setup_api.sh— Network exposure0.0.0.0(unauthenticated, all interfaces) →127.0.0.1stealth_post.sh— Credential exposure in process list--passphrase "$GPG_PASSPHRASE"→printf '%s' "$GPG_PASSPHRASE" | gpg --passphrase-fd 0--user user:pass→ tempchmod 600netrc file (--netrc-file), included inshred/trap cleanuppentest_verification.sh— Default credentials + XML injection${GVM_USER:-admin}/${GVM_PASSWORD:-admin}; script now fails fast when these are unset^[A-Za-z0-9._-]+$) before interpolating$hostintogvm-cli --xmlpentest_exploitation.sh— Dry-run side-effect + missing arg validation": >$SEARCH_FILE"(file truncation) was unconditional; guarded by[[ "$DRY_RUN" != true ]]--resultsand--search-fileSharePointManagement.ps1— Scope bug + missing validationAdd-Userreferenced undefined$SPSiteinstead of$script:SPSiteConnect-SPnow throws on emptySiteUrlfor both Online and OnPrem modes with-ErrorAction StopSiteUrl/TemplateforCreateSite;UserLoginforAddUserExchangeOnlineManagement.ps1— Silent failure + unhandled errorsreturn(exit code 0) →exit 1try/catchtolistanddisconnectactionsValidation
--dry-runor-WhatIfSafety