✨ Add Support for Environment Flags NO_COLOR and FORCE_COLOR

[aspiers]

🕓 Changelog

This PR adds support for the environment flags NO_COLOR and FORCE_COLOR to control the colour output. Specifically, the colour output is auto-detected and can be controlled with:

• NO_COLOR=true — disables all colours,
• FORCE_COLOR=true — forces colour output.

Only the exact value true is accepted to avoid accidental activation.

~$ NO_COLOR=true ./safe_hashes.sh ...
~$ FORCE_COLOR=true ./safe_hashes.sh ...

If both are set, NO_COLOR takes precedence and disables all formatting. Otherwise, colour is enabled only if output is to a terminal, tput is available, and the terminal supports at least the 8 standard ANSI colours.

[pcaversaccio]

thx for the PR @aspiers - could you clarify why this change would be necessary for the vast majority of users? I'm generally hesitant to introduce new CLI options unless they provide clear benefits for 99% of users. Even optional features add complexity and require users to consider their implications. Given that this is mission-critical software, I want to avoid feature creep and keep the CLI as streamlined as possible.

[pcaversaccio]

Also, I refuse to call this --color; only "colour" is the right "colour" 😄

[aspiers]

@pcaversaccio commented on Apr 2, 2025, 3:06 PM GMT+1:

Also, I refuse to call this --color; only "colour" is the right "colour" 😄

Originally posted by @pcaversaccio in #33 (comment)

I'm a Brit, so colour is my preferred spelling too ;-) My default assumption is that the majority of software projects out there using the English language use US spellings, which is why I tend to use color in PRs. But I've changed it to colour! 😆

@pcaversaccio commented on Apr 2, 2025, 2:58 PM GMT+1:

thx for the PR - could you clarify why this change would be necessary for the vast majority of users?

Sure. The most obvious use case is piping the output through a pager and/or other filter. For example, I find it extremely useful to filter through the rolod0x CLI, so that human-readable labels are attached to addresses as an extra layer of security.

Clearly you already appreciate that colour is an extremely effective way of improving the usability of UIs, otherwise you wouldn't have gone to the effort of supporting colour in the first place. And it's no less useful with the above use cases than any other use case. So I think it doesn't make sense to exclude the benefits of colour from these use cases.

BTW I did a quick scan of the man pages on my system and found over 50 CLI programs which support this approach: aha bison rpm-misc csh fluxbox cfdisk fdisk sfdisk diff grep cargo cc c++ gcc git g++ rustc dir ls vdir cal dmesg hexdump parallel tcsh node21 msgconv msgcomm msgcat msgattrib xgettext msguniq msgunfmt msgmerge msginit msggrep msgfilter msgen valac-0.56 valac rg fd bat quilt shellcheck brz pw-config pw

I'm generally hesitant to introduce new CLI options unless they provide clear benefits for 99% of users. Even optional features add complexity and require users to consider their implications. Given that this is mission-critical software, I want to avoid feature creep and keep the CLI as streamlined as possible.

I understand where you're coming from, but the benefits of colour are already well established as per above, and it hardly makes any difference to the complexity of the code. After splitting out the refactoring into a separate commit, the new functionality is only 4 extra lines of actual logic, and 12 extra lines for parsing the new option, all of which are extremely simple and cleanly separated from the rest of the logic.

[pcaversaccio]

I'm a Brit, so colour is my preferred spelling too ;-) My default assumption is that the majority of software projects out there using the English language use US spellings, which is why I tend to use color in PRs. But I've changed it to colour! 😆

There is English and American English 😄!

Sure. The most obvious use case is piping the output through a pager and/or other filter. For example, I find it extremely useful to filter through the rolod0x CLI, so that human-readable labels are attached to addresses as an extra layer of security.

The use case for this script is to run it on a dedicated device running a secure operating system. Piping, filtering etc. create additional friction points and thus attack vectors that can be manipulated. For simple piping, you can do something like here (convert the output into a simple JSON instead). I appreciate your feedback, but I still don't understand why anyone needs this feature.

Also, adding this feature would be a bit more complicated, as I manually colour things (mostly warnings) later in the script, e.g.

safe-tx-hashes-util/safe_hashes.sh

Lines 380 to 396 in 88e2916

	# Check if the called function is sensitive and print a warning in bold.
	case "$method" in
	addOwnerWithThreshold | removeOwner | swapOwner | changeThreshold)
	echo
	echo -e "${BOLD}${RED}WARNING: The \"$method\" function modifies the owners or threshold of the Safe. Proceed with caution!${RESET}"
	;;
	esac
	# Check for sensitive functions in nested transactions.
	echo "$parameters" | jq -c ".[] | .valueDecoded[]? | select(.dataDecoded != null)" | while read -r nested_param; do
	nested_method=$(echo "$nested_param" | jq -r ".dataDecoded.method")
	if [[ "$nested_method" =~ ^(addOwnerWithThreshold|removeOwner|swapOwner|changeThreshold)$ ]]; then
	echo
	echo -e "${BOLD}${RED}WARNING: The \"$nested_method\" function modifies the owners or threshold of the Safe! Proceed with caution!${RESET}"
	fi
	done

safe-tx-hashes-util/safe_hashes.sh

Lines 636 to 645 in 88e2916

	# Warn the user if `operation` equals `1`, implying a `delegatecall`, and if the `to` address is untrusted.
	# See: https://github.com/safe-global/safe-smart-account/blob/34359e8305d618b7d74e39ed370a6b59ab14f827/contracts/libraries/Enum.sol.
	if [[ "$operation" -eq 1 && ! " ${TRUSTED_FOR_DELEGATE_CALL[@]} " =~ " ${to} " ]]; then
	cat <<EOF
	$(tput setaf 1)WARNING: The transaction includes an untrusted delegate call to address $to!
	This may lead to unexpected behaviour or vulnerabilities. Please review it carefully before you sign!$(tput sgr0)
	EOF
	delegate_call_warning_shown="true"

safe-tx-hashes-util/safe_hashes.sh

Lines 918 to 926 in 88e2916

	$(tput setaf 1)If it's not already obvious: This is YOLO mode – BE VERY CAREFUL!$(tput sgr0)
	$(tput setaf 3)IMPORTANT:
	- Leaving a parameter empty will use the value retrieved from the Safe transaction service API, displayed as the "default value".
	If the value is unavailable (e.g. if the API endpoint is down), it will default to zero.
	- If multiple transactions share the same nonce, the first transaction in the array will be selected to provide the default values.
	- No warnings will be shown if multiple transactions share the same nonce. It's recommended to first run a validation without interactive mode enabled!
	- Some parameters (e.g., \`version\`, \`to\`, \`operation\`) enforce valid options, but not all inputs are strictly validated.
	Please double-check your entries before proceeding.$(tput sgr0)

The primary reason I didn't properly refactor this yet is that it doesn't matter for the use case of this script.

I understand where you're coming from, but the benefits of colour are already well established as per above, and it hardly makes any difference to the complexity of the code. After splitting out the refactoring into a separate commit, the new functionality is only 4 extra lines of actual logic, and 12 extra lines for parsing the new option, all of which are extremely simple and cleanly separated from the rest of the logic.

Well, as I explained above, it would require more refactoring for all colourings used. Also, I follow the YAGNI principle for such mission-critical software, so every line of code must be justified for the use case, and it doesn't matter if it's a best practice in other tools.

[aspiers]

Firstly, thanks a lot for the continued replies. I do appreciate it. Hopefully you will also gain something useful from this conversation, even if it's not a merged PR 😉

I'm a Brit, so colour is my preferred spelling too ;-) My default assumption is that the majority of software projects out there using the English language use US spellings, which is why I tend to use color in PRs. But I've changed it to colour! 😆

There is English and American English 😄!

Indeed!

Sure. The most obvious use case is piping the output through a pager and/or other filter. For example, I find it extremely useful to filter through the rolod0x CLI, so that human-readable labels are attached to addresses as an extra layer of security.

The use case for this script is to run it on a dedicated device running a secure operating system.

That's clearly a very valid use case, and I appreciate it's the one you've documented, but it's definitely not my use case, for carefully considered reasons explained below. And if I were a betting man I would put money on there being others who run it on normal operating systems on multi-purpose machines for similar reasons.

If this alarms you, it shouldn't! The famous essay "The Cathedral and the Bazaar" states:

Any tool should be useful in the expected way, but a truly great tool lends itself to uses you never expected.

The best practices you document in the README are of course excellent advice for higher stakes scenarios, and arguably essential when there is (say) >$1m at risk. But best practices by definition are guidelines not universally applicable rules. Everyone has different security requirements and should be free to choose where they want to sit on the security/convenience trade-off spectrum for any given scenario.

For example, only verifying Safe transactions from a dedicated machine could easily be considered overkill if there's less than $1k of funds at risk in an organization which regularly deals with amounts in excess of $10m. (The threshold could of course be higher or lower depending on the size of the organization and many other factors.) Or similarly if the cost of an attack is many orders of magnitude higher than the value to the attacker. Many Safe transactions don't even involve any funds at all. And catering for different security requirements is of course why Safe allows the owner to choose the signing threshold.

Piping, filtering etc. create additional friction points and thus attack vectors that can be manipulated.

Sure, but I trust the script I'm piping it through at least as much as all the other attack surfaces involved, because I wrote that code myself. And anyway, the extra security guarantees offered to me by rolod0x easily outweigh the risks of it being compromised. Or if I piped it through less, I would trust the less binary much more than I trust the Foundry install itself. These two examples should further demonstrate why best practices like "be very careful what you pipe security-critical data through" make more sense than hard rules like "NEVER pipe security-critical data through anything, ever!"

(BTW, a tangential point here is that one of the trust assumptions required by the script is that the local Foundry install is not compromised. Unfortunately, Foundry's officially documented installation procedure is curl -L https://foundry.paradigm.xyz | bash, which is known to be an extremely poor security practice. I worked at SUSE for ~15 years alongside many experts in the realm of software distribution, and heard several of my colleagues voice their frustration as this pattern gained popularity, overlooking decades of learned best practices on how to securely package and distribute Open Source. At least the Foundry book recommends verifying integrity and provenance of binaries, but that introduces a further dependency on gh.)

The more general point here is the script cannot possibly know the security context in which it is run, let alone the relevant merits and risks of any other program to which its output may be piped. Therefore it does not make sense for the script to assume any of that context.

For simple piping, you can do something like here (convert the output into a simple JSON instead).

Unfortunately that doesn't help me at all, because outputting in JSON (or any other format) is of no use to me.

I appreciate your feedback, but I still don't understand why anyone needs this feature.

Hopefully it's clear from the above now? For carefully considered reasons, my security requirements are less stringent than yours in many circumstances. And in those cases I simply want the output of the script to be as human-readable as possible so that I can process a large number of low criticality transactions without undue brain fatigue, which means I need coloured output where addresses are automatically labelled by rolod0x.

Also, adding this feature would be a bit more complicated, as I manually colour things (mostly warnings) later in the script, e.g.

safe-tx-hashes-util/safe_hashes.sh

Lines 380 to 396 in 88e2916

	# Check if the called function is sensitive and print a warning in bold.
	case "$method" in
	addOwnerWithThreshold | removeOwner | swapOwner | changeThreshold)
	echo
	echo -e "${BOLD}${RED}WARNING: The \"$method\" function modifies the owners or threshold of the Safe. Proceed with caution!${RESET}"
	;;
	esac
	# Check for sensitive functions in nested transactions.
	echo "$parameters" | jq -c ".[] | .valueDecoded[]? | select(.dataDecoded != null)" | while read -r nested_param; do
	nested_method=$(echo "$nested_param" | jq -r ".dataDecoded.method")
	if [[ "$nested_method" =~ ^(addOwnerWithThreshold|removeOwner|swapOwner|changeThreshold)$ ]]; then
	echo
	echo -e "${BOLD}${RED}WARNING: The \"$nested_method\" function modifies the owners or threshold of the Safe! Proceed with caution!${RESET}"
	fi
	done

safe-tx-hashes-util/safe_hashes.sh

Lines 636 to 645 in 88e2916

	# Warn the user if `operation` equals `1`, implying a `delegatecall`, and if the `to` address is untrusted.
	# See: https://github.com/safe-global/safe-smart-account/blob/34359e8305d618b7d74e39ed370a6b59ab14f827/contracts/libraries/Enum.sol.
	if [[ "$operation" -eq 1 && ! " ${TRUSTED_FOR_DELEGATE_CALL[@]} " =~ " ${to} " ]]; then
	cat <<EOF
	$(tput setaf 1)WARNING: The transaction includes an untrusted delegate call to address $to!
	This may lead to unexpected behaviour or vulnerabilities. Please review it carefully before you sign!$(tput sgr0)
	EOF
	delegate_call_warning_shown="true"

safe-tx-hashes-util/safe_hashes.sh

Lines 918 to 926 in 88e2916

	$(tput setaf 1)If it's not already obvious: This is YOLO mode – BE VERY CAREFUL!$(tput sgr0)
	$(tput setaf 3)IMPORTANT:
	- Leaving a parameter empty will use the value retrieved from the Safe transaction service API, displayed as the "default value".
	If the value is unavailable (e.g. if the API endpoint is down), it will default to zero.
	- If multiple transactions share the same nonce, the first transaction in the array will be selected to provide the default values.
	- No warnings will be shown if multiple transactions share the same nonce. It's recommended to first run a validation without interactive mode enabled!
	- Some parameters (e.g., \`version\`, \`to\`, \`operation\`) enforce valid options, but not all inputs are strictly validated.
	Please double-check your entries before proceeding.$(tput sgr0)

Ah yes, good point. Well I could easily address those, if you're willing to reconsider your position here.

The primary reason I didn't properly refactor this yet is that it doesn't matter for the use case of this script.

Sure, that refactoring definitely wasn't needed until I came along with my use case.

Well, as I explained above, it would require more refactoring for all colourings used. Also, I follow the YAGNI principle for such mission-critical software, so every line of code must be justified for the use case, and it doesn't matter if it's a best practice in other tools.

Please consider who the "You" refers to in this principle. Just because you don't need it, it doesn't mean that other users of the script don't need it. I'm one such user saying that I do need it (otherwise I wouldn't have bothered with this PR). And as per esr's quote from the CatB essay above, if your users' needs are evolving beyond your expectations, that means you're doing something right 😎

[pcaversaccio]

Thanks for the insights - while I have to think about my position here, I wanted to ask you what you think about using NO_COLOR here instead? For example:

NO_COLOR=1 ./safe_hashes.sh --network arbitrum --address 0x111CEEee040739fD91D29C34C33E6B3E112F2177 --nonce 234

would disable colour by default in the output. NO_COLOR is an accepted standard and I personally use it also in combination with forge frequently (see, e.g., here).

I really don't want to overload the CLI options and I already implement an environment variable DEBUG=true in the script.

[aspiers]

I would be totally fine with a solution which uses an environment variable rather than a new CLI option. I only suggested the CLI option because --color=<never|always|auto> is already in common usage across many well-known programs.

I did consider mentioning NO_COLOR, but that standard is purely for disabling colour, which is the opposite of what I want: i.e. forcing colour even when STDOUT is not a tty. It's a shame that https://no-color.org/ not only omitted dealing with this use case, but also didn't leave space for an extension of the standard which could cleanly accommodate it whilst maintaining compliance / backwards compatibility. For example, something like NO_COLOR=never is semantically equivalent to --color=always, but their spec says "regardless of its value" which requires that never is treated identically to 1 or true.

That said, while it's perhaps slightly controversial to violate the spec, personally I think it would be a reasonable compromise to write code which treats NO_COLOR=never as a special case yielding the opposite to NO_COLOR=1. This is because I highly doubt anyone would set NO_COLOR to never while expecting it to result in colour being disabled. And it seems even less likely that colour being unexpectedly enabled in such a scenario would cause anyone any significant harm.

[pcaversaccio]

Hmm - what about we use FORCE_COLOR? I know it's an informal standard, but probably exactly what we need here.

[aspiers]

Sure, sounds fine to me!

[aspiers]

New version pushed.

[pcaversaccio]

@aspiers I refactored the version a bit in 248ef87. Still needs proper testing, but would you agree with the design? Fwiw, I also updated the PR description. Maybe an open question is what takes precedence.

[aspiers]

Looks great to me! The replacement of all those tput commands also greatly increases the legibility of the source.

Agreed, it's unclear which should take precedence, but I highly doubt it matters - and if it did then it would be trivial to fix anyway.

[pcaversaccio]

Agreed, it's unclear which should take precedence, but I highly doubt it matters - and if it did then it would be trivial to fix anyway.

I think the current approach is the most sound one. It's like having both a mute button and a volume slider:

• FORCE_COLOR is the turned up volume control.
• NO_COLOR is mute.
• Mute always wins.

[pcaversaccio]

Ok tested it extensively - works as expected; thx for the PR and the insightful discussion 😄!

[aspiers]

Awesome! 🚀 Thanks a lot for your time on this! 🙏