Repository navigation
land: command-redaction-output - #854
Open
aft-alfonso[bot] wants to merge 10 commits into
Open
aft-alfonso[bot] wants to merge 10 commits into
aft-alfonso[bot] wants to merge 10 commits into
Conversation
…low-ups (#825, #759) Tests only, all ignored with "fails today (#N)" reasons; each fails today for the stated reason when run with --ignored: - spawn_log_masks_inline_command_credentials (#825): the background spawn log line, after the same redactor the log file writer applies, still carries NAME=value secrets, credential flags, mysql -p<value>, and a GitHub token cut short by the 200-character limit. Also fences seven ordinary commands (cargo -p, git log -p, ssh -p, TOKENIZERS_PARALLELISM, GIT_AUTHOR_NAME, $VAR references, gcloud --secret=<name>) that any broader rule must leave unchanged. - reply_cap_keeps_failure_lines_from_the_middle_of_unrecognised_output (#759): the reply byte cap drops FAILED:/ERROR: lines from the middle of output no compressor recognises. - routine_out_of_root_cat_decline_is_not_a_warning (#759): every cat of a file outside the project root writes a WARN line. - not_found_error_does_not_repeat_a_large_match in batch and edit_match (#759): the not-found error pastes the whole match text back. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Nothing reads compression_events.command (aggregates, status and retention use counts and token sums only), and a stored command can carry an inline credential. New events no longer write it: CompressionEventRow loses its command field and the insert leaves the column NULL. Rows written by earlier builds are cleared once per database on a writable open (UPDATE ... SET command = NULL), recorded by a host_state marker so later opens only read the marker. The column stays and the schema version is unchanged, so older builds can still open aft.db; read-only opens never run the cleanup. The integration test compression_event_records_command_for_diagnostics pinned the stored command; nothing outside that test read it. It becomes compression_event_does_not_store_the_command and asserts the opposite through the real completion path, which is the point of this change. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
The 'bash spawned' INFO line quoted the first 200 characters of the command with only the shared log patterns applied, so GITHUB_TOKEN=..., export DB_PASSWORD=..., --password/--token values and mysql -p<value> reached the log file and the plugin log copy. It also cut before masking, so a token crossing the 200-character limit lost the length its pattern needs and was written partly unmasked. log_redact::redact_command adds command-only shapes on top of aft_redactor: UPPER_SNAKE secret-named assignments (skipping *_PATH/_FILE/_ID/... names and $VAR / $(...) references), credential long options (not --secret, which names a secret), and tool-scoped mysql -p<value>, sshpass -p and curl -u user:pass. Generic -p flags (cargo, ssh, mkdir) are untouched. log_bash_spawn masks the whole command, then shortens it. The fixture holds the 24 masked cases and 803 untouched commands (every command line from this repo's scripts, workflows and package.json scripts plus common cargo/git/npm/ssh/docker invocations); the command shapes change none of them. log_bash_spawn is the only log site that quotes the shell command. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
A command no compressor recognises gets only the 16 KiB reply cap: a 6 KiB head, at most twenty tail lines and each stream's final line. Failure lines in between were dropped (3,000 lines with FAILED: at 1500 and ERROR: at 2200 showed neither), because the compressor-level failure rescue runs only after a specific compressor matched, and before this cap. For a known non-zero exit the reply cap now keeps up to twenty failure lines (4 KiB) from the cut middle, first ones first, under the existing '[raw failure lines preserved by AFT]' label, just before the final line. The scan stops at the twentieth line. The block's bytes come out of the head budget, and the lines are picked again from the resulting cut (at most three passes) so the reply stays within its cap. Successful commands, completion previews and raw (uncompressed) replies are unchanged. The label text moves to compress::PRESERVED_FAILURES_LABEL, shared by both rescues; the compressor-level output is byte-identical. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
A failed edit's not-found error pasted the whole searched text back (batch: edit[i] match '<text>' not found in file, and both edit_match forms), so a large oldString was repeated into the transcript. The host edit tool reaches the batch form for a single file. fuzzy_match::describe_searched_text quotes one line of up to 120 characters as before; anything longer becomes its first line cut to 120 characters, an ellipsis and its size, e.g. ' let absent_0 = 0;…' (400 lines, 9490 bytes) The bounded nearest-miss detail after it is unchanged. large_match_reflow_search_is_linear_and_still_applies asserted the full message including its 64/256 KiB needle; it now expects the short form, which is the point of this change. Short-match assertions elsewhere are unchanged. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Every cat of a file outside the project root (a host's temp folder such as $TMPDIR/opencode/...) or of an unreadable shape logged a WARN line, although the command just runs as native bash. The decline line keeps its predicate and resolved path at debug, beside the dispatcher's own debug record. The two tests that pinned the line now capture at debug; the one named 'logs_warning' is renamed to 'logs_decline'. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ommands (#825) Two gaps in redact_command: - The assignment rule required a letter before the credential word, so a bare TOKEN=hunter2 or PASSWORD=hunter2 was logged unmasked. The name may now start with the credential word; TOKENIZERS_PARALLELISM and MAX_TOKENS still do not match, because the word must be followed by = or _. - Values stopped inside a shell word: curl -u "admin:my password" masked only 'my', the mysql and sshpass rules stopped at a quoted space, and the assignment and long-option rules stopped at an escaped quote or after the first part of a concatenation ('a'"b"c). Every value pattern now takes the whole shell word: single-quoted, double-quoted with escapes, backslash-escaped and concatenated runs, up to the next unquoted whitespace or shell operator. The curl rule takes the whole user:password word and masks what follows the first colon, keeping a wrapping quote pair. Eleven masked fixtures cover these forms (35 in all); the 803 benign commands, including cargo test -p and mkdir -p, are still unchanged. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
The curl rule matched the first -u value after the curl prefix. A username-only value (curl -u alice https://one --next -u admin:hunter2 https://two) was left unchanged, but the match had consumed the only curl prefix, so the later admin:hunter2 was never checked and reached the log. The rule now takes each curl command span (up to the next shell operator or newline) and checks every user option inside it, in all of curl's spellings: -u value, -uvalue, --user value and --user=value. A value without a password, or with a reference as its password, is still left as written. Fixtures add a --next command with a later password, two -u values that both have passwords, --user=admin:pw and -uadmin:pw (39 masked cases); username-only values join the untouched list, which stays unchanged. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…825) The curl command span (\bcurl\b[^;&|\n]*) and the mysql prefix ([^;&|\n]*?) stopped at the first ;, & or |, even inside quotes. In curl -u "admin:pa;ss" https://x the span ended inside the quoted value, so only 'pa' was masked and ';ss"' stayed in the log. Both are now built from whole shell words separated by spaces or tabs (shell_word!), so an operator or newline ends the command only outside quotes; redirections and parentheses do not end it. sshpass takes its value directly after -p and already used whole words. Fixtures add quoted passwords containing ;, & and | for curl, mysql (also after a quoted host containing ;) and sshpass, and a curl that follows an unquoted ; (45 masked). Two commands whose unquoted ; really ends the curl or mysql command before a later -u or -p join the untouched list. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
| /// quotes. Redirections and parentheses do not end it. | ||
| macro_rules! command_args { | ||
| () => { | ||
| concat!(r"(?:[ \t]+(?:", shell_word!(), r"|[<>()])+)*") |
There was a problem hiding this comment.
Commands such as curl</dev/null -u admin:hunter2 https://host now leave the password in the log. command_args! requires whitespace immediately after curl, so it stops before the redirection and never reaches -u. The changed COMMAND_MYSQL_PASSWORD has the same problem with mysql</dev/null -u root -phunter2 db. Both forms were covered by the previous patterns.
Allow attached redirections before later arguments and add fixtures for both commands.
How this was verified: Both changed patterns stop at the attached redirection, and the remaining plain password passes through log_bash_spawn and the shared redactor into the log file and stderr.
Knowledge Base Used: Runtime safety and operations
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.
Accepted agent delivery; branch-keyed search descriptor included.
The password leak in
log_redact.rsshould be fixed before merging.Findings
Summary
The PR reduces command text kept in diagnostics and improves shortened replies.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Shell command] --> B[Mask inline credentials] B --> C[Shorten spawn log] C --> D[Shared log redactor] D --> E[Log file] D --> F[Stderr]Reviews (3) · Last reviewed commit: "mason: end logged curl and mysql command..." · Reviewed by Greptile