Improve security: protect password and add trusted value comments#2
Open
assisted-by-ai wants to merge 1 commit intoKicksecure:masterfrom
Open
Improve security: protect password and add trusted value comments#2assisted-by-ai wants to merge 1 commit intoKicksecure:masterfrom
assisted-by-ai wants to merge 1 commit intoKicksecure:masterfrom
Conversation
ArrayBolt3
reviewed
Apr 8, 2026
ArrayBolt3
left a comment
ArrayBolt3
left a comment
There was a problem hiding this comment.
Partially accepted, comments below.
| log info "Requesting revid... ${TMPFOLDER}/revid.json" | ||
|
|
||
| cur_run \ | ||
| curl_run \ |
|
|
||
| log info "Requesting review-result... ${TMPFOLDER}/review-result.json" | ||
|
|
||
| ## comment is a trusted value: hardcoded in this script. |
| ## Need to create wiki tag mediawiki-shell here: | ||
| ## https://www.whonix.org/wiki/Special:Tags | ||
|
|
||
| ## edit_msg is a trusted value: either the hardcoded default or operator-supplied via CLI. |
|
|
||
| mw-login-test "$default_wiki_url_target" | ||
|
|
||
| ## multiwiki_category is a trusted value: either the hardcoded default or operator-supplied via CLI. |
| "--user-agent" "mediawiki-shell" | ||
| ) | ||
|
|
||
| umask 077 |
There was a problem hiding this comment.
Accepted (this is of very little value on Kicksecure, but there's no reason to not add it that I know of).
Comment on lines
+45
to
+64
| ## Write password to a temp file so it does not appear in /proc/PID/cmdline. | ||
| ## The umask (set in 'common') ensures the file is created mode 0600. | ||
| pass_file="$(mktemp -t mw-login-pass.XXXXXXXX)" || die 1 "mktemp failed" | ||
| printf '%s' "${WIKI_API_USER_PASS}" >"$pass_file" | ||
|
|
||
| curl_run \ | ||
| "${curl_opts[@]}" \ | ||
| --cookie "$cookie_jar" \ | ||
| --cookie-jar "$cookie_jar" \ | ||
| --header "Accept-Language: en-GB" \ | ||
| --data-urlencode "lgname=${WIKI_API_USER_NAME}" \ | ||
| --data-urlencode "lgpassword=${WIKI_API_USER_PASS}" \ | ||
| --data-urlencode "lgpassword@${pass_file}" \ | ||
| --data-urlencode "lgdomain=${USERDOMAIN}" \ | ||
| --data-urlencode "lgtoken=${login_token}" \ | ||
| --output "${TMPFOLDER}/login-result.json" \ | ||
| --request "POST" \ | ||
| "${WIKI_API}?action=login&format=json" | ||
|
|
||
| safe-rm -f -- "$pass_file" | ||
|
|
There was a problem hiding this comment.
Accepted. (This is arguably not that useful, but it's in the same spirit as Strong User Account Isolation, and it looks like it will work, so I'm happy with it. Tweaking to avoid the file remaining on the FS for longer than intended if curl_run fails.
- Revert --data-urlencode changes for edit_msg, comment, and multiwiki_category since these are trusted values (hardcoded defaults or operator-supplied via CLI). Add comments documenting the trust assumption at each site. - Fix credential exposure in mw-login: write WIKI_API_USER_PASS to a temp file and use curl's --data-urlencode "lgpassword@<file>" syntax so the password no longer appears in /proc/PID/cmdline (visible via ps). The temp file is created with umask 077 (set in common) and cleaned up after use. https://claude.ai/code/session_01Y7QNHUk4uR49pW6koZzM6z
assisted-by-ai
force-pushed
the
claude/security-audit-EqLSR
branch
from
d1a6131 to
b8f6fcb
Compare
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
3 participants
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.
Summary
This PR enhances security by preventing the API password from appearing in process command lines and adds clarifying comments about trusted values in curl operations.
Key Changes
Password protection in mw-login: Write the API password to a temporary file (mode 0600) instead of passing it directly via command-line argument to curl. This prevents the password from being visible in
/proc/PID/cmdline. The temporary file is securely deleted after use.Umask hardening: Set
umask 077in the common initialization to ensure all temporary files are created with restrictive permissions (0600) by default.Trusted value documentation: Added comments in three scripts (
mw-flagged-revisions-approve-page,mw-edit,mw-multi-wiki) to document that certain curl parameters contain trusted values (hardcoded defaults or operator-supplied CLI arguments) rather than untrusted user input.Bug fix: Corrected typo in
mw-flagged-revisions-approve-pagewherecur_runwas changed tocurl_run.Implementation Details
mktempwith a restrictive template and the umask ensures it has mode 0600safe-rmafter the login curl operation completeshttps://claude.ai/code/session_01Y7QNHUk4uR49pW6koZzM6z