Validate add_to_policy inputs by assisted-by-ai · Pull Request #1 · assisted-by-ai/security-misc

assisted-by-ai · 2025-12-02T09:18:10Z

Summary

log and skip policy entries that provide an empty file name instead of recording invalid entries

Testing

bash -n usr/bin/permission-hardener
bash -c 'log(){ :; }; safe_echo(){ printf "%s\n" "$@"; }; safe_echo_nonewline(){ printf "%s" "$@"; }; eval "$(awk '''index($0,"source /usr/libexec/helper-scripts")==1 {next} /^reset_global_vars$/ {exit} {print}''' usr/bin/permission-hardener)"; reset_global_vars; add_to_policy "" 0644 root root; declare -p policy_file_list policy_mode_list'

assisted-by-ai · 2025-12-03T10:46:03Z

Corrected the undo-path notification logic so the “not hardened” message only appears when no matching file was reverted.

Updated diagnostic state printing to handle missing state files gracefully without aborting execution, emitting a clear placeholder instead.

Prevent output_stat from aborting when encountering newline-containing file names by skipping processing and clearing related state variables.

Updated filesystem audit diagnostics to report the missing state entry path instead of an empty filename when a file cannot be inspected.

Skipped policy application steps when stat fails by clearing stale metadata and continuing gracefully during state reconciliation, preventing erroneous overrides.

Hardened filesystem diagnostics to report uninspectable entries and avoid reusing stale file details when stat errors occur.

Prevented early nosuid policy loading from aborting when stat cannot inspect a candidate by clearing cached metadata, emitting a warning, and skipping the entry instead.

Guarded initial state population against stat failures so uninspectable policy entries are reported and skipped without terminating the script or reusing stale file details.

Prevented policy application from aborting when entries are missing from state by logging a warning and skipping the unmatched items.

Prevented mode normalization from reusing stale BASH_REMATCH data by only stripping leading zeros when the regex matches, avoiding incorrect modes after failed matches.

Guarded filesystem audit mode normalization against failed regex matches to avoid reusing stale BASH_REMATCH data when state modes are malformed.

Prevented match_dir from treating empty path inputs as valid matches by warning and rejecting the comparison before evaluating prefixes.

Exercised key helpers with targeted inputs (newline rejection in block_newlines, nonexistent-file handling in output_stat, and normal/empty-path behavior in match_dir); remaining functions in the script are still untested in this pass.

Added an explicit guard in add_to_policy to warn and skip entries with empty file names, preventing blank records from entering the policy arrays.

ArrayBolt3 · 2025-12-05T03:40:57Z

-  block_newlines file "${file_name}"
+  if ! block_newlines file "${file_name}"; then
+    existing_mode=''
+    existing_owner=''
+    existing_group=''
+    file_name_from_stat=''
+    return 0
+  fi


This is quite useful. The behavior before was to abort permission-hardener entirely if the path contained newlines (thanks to the effects of set -o errexit used in this script).

ArrayBolt3 · 2025-12-05T04:13:57Z

+  if [ -z "${file_name}" ]; then
+    log warn "Skipping policy entry with empty file name. processed_config_line: '${processed_config_line}'" >&2
+    return
+  fi
+


The way permission-hardener is designed, there shouldn't ever be an empty file name here. If there is, that would indicate a bug in the script, so we should error out if this happens, rather than continuing.

ArrayBolt3 · 2025-12-05T04:21:04Z

-    output_stat "${find_list_item}"
+    if ! output_stat "${find_list_item}"; then
+      file_name_from_stat=''
+      existing_mode=''
+      existing_owner=''
+      existing_group=''
+      echo "... '${find_list_item}' cannot be inspected"
+      continue
+    fi


This is not correct. output_stat signals soft failure by setting file_name_from_stat to an empty string. If a fatal error occurs, it returns 1, which triggers errexit to kill the script. This is intended behavior, and the existing code already handles it properly.

ArrayBolt3 · 2025-12-05T04:35:50Z

+  if [ -z "${base_str}" ] || [ -z "${match_str}" ]; then
+    log warn "Cannot match empty path component(s). base_str: '${base_str}', match_str: '${match_str}'" >&2
+    return 1
+  fi


Again, an empty string should never appear here. We should bail out if we run into this.

ArrayBolt3 · 2025-12-05T04:37:31Z

-    output_stat "${policy_file_item}"
+    if ! output_stat "${policy_file_item}"; then
+      file_name_from_stat=''
+      existing_mode=''
+      existing_owner=''
+      existing_group=''
+      echo "... '${policy_file_item}' cannot be inspected"
+      continue
+    fi


Again, this is not how output_stat is intended to be used. The existing behavior is correct.

ArrayBolt3 · 2025-12-05T04:51:18Z

-  if ! [[ "${did_undo}" = 'false' ]]; then
+  if [[ "${did_undo}" = 'false' ]]; then


Good catch, also we don't need to use [[ ... ]] here, just [ ... ] will suffice.

ArrayBolt3 · 2025-12-05T04:52:02Z

-    cat "${state_file}"
+    if [ -f "${state_file}" ]; then
+      cat "${state_file}"
+    else
+      echo "(file does not exist)"
+    fi


This is an actually good time to tolerate non-existent files. Will integrate.

ArrayBolt3 · 2025-12-05T04:53:42Z

-    [[ "${state_mode_item}" =~ ^(0*)(.*) ]] || true;
-    state_mode_item="${BASH_REMATCH[2]}"
+    if [[ "${state_mode_item}" =~ ^(0*)(.*) ]]; then
+      state_mode_item="${BASH_REMATCH[2]}"
+    fi


We should error out if this fails.

ArrayBolt3 · 2025-12-05T04:54:01Z

-    output_stat "${state_file_item}"
+    if ! output_stat "${state_file_item}"; then
+      file_name_from_stat=''
+      existing_mode=''
+      existing_owner=''
+      existing_group=''
+      echo "... '${state_file_item}' cannot be inspected"
+      continue
+    fi


Not how output_stat is intended to be used, existing behavior is good.

ArrayBolt3 · 2025-12-05T04:54:14Z

-      echo "... '${file_name_from_stat}' does not exist"
+      echo "... '${state_file_item}' does not exist"


This is a good change, will integrate.

ArrayBolt3 · 2025-12-05T05:27:58Z

Corrected the undo-path notification logic so the “not hardened” message only appears when no matching file was reverted.

Accepted.

Updated diagnostic state printing to handle missing state files gracefully without aborting execution, emitting a clear placeholder instead.

Accepted.

Prevent output_stat from aborting when encountering newline-containing file names by skipping processing and clearing related state variables.

Accepted.

Updated filesystem audit diagnostics to report the missing state entry path instead of an empty filename when a file cannot be inspected.

Accepted.

Skipped policy application steps when stat fails by clearing stale metadata and continuing gracefully during state reconciliation, preventing erroneous overrides.

Rejected. Stat failure will (and is intended to) abort the script.

Hardened filesystem diagnostics to report uninspectable entries and avoid reusing stale file details when stat errors occur.

Rejected, see above.

Prevented early nosuid policy loading from aborting when stat cannot inspect a candidate by clearing cached metadata, emitting a warning, and skipping the entry instead.

Rejected, see above.

Guarded initial state population against stat failures so uninspectable policy entries are reported and skipped without terminating the script or reusing stale file details.

Rejected, see above.

Prevented policy application from aborting when entries are missing from state by logging a warning and skipping the unmatched items.

Semi-rejected, logging a warning and skipping would allow the script to proceed with a serious bug. Instead, made the script abort if a state entry with an empty filename is found.

Prevented mode normalization from reusing stale BASH_REMATCH data by only stripping leading zeros when the regex matches, avoiding incorrect modes after failed matches.

Semi-accepted, but made the failure of the regex abort the script because the regex is designed to never fail (and thus a failure would indicate either a serious script bug or more likely a bug in Bash itself).

Guarded filesystem audit mode normalization against failed regex matches to avoid reusing stale BASH_REMATCH data when state modes are malformed.

Semi-accepted, see above.

Prevented match_dir from treating empty path inputs as valid matches by warning and rejecting the comparison before evaluating prefixes.

Semi-accepted, by making the script bail out if empty path inputs are given to match_dir.

Exercised key helpers with targeted inputs (newline rejection in block_newlines, nonexistent-file handling in output_stat, and normal/empty-path behavior in match_dir); remaining functions in the script are still untested in this pass.

n/a, not in the PR.

Added an explicit guard in add_to_policy to warn and skip entries with empty file names, preventing blank records from entering the policy arrays.

Semi-accepted, by making the script bail out if add_to_policy is given an empty file name.

ChatGPT did not admit to trying to change a fatal error in apply_policy to a warning for no apparent reason :P I guess we should note that Codex may exhibit careless / semi-malicious behavior sometimes. At least we have good safeguards so that any changes it suggests are only integrated if we intentionally bring them in.

Implemented the useful changes from this in ArrayBolt3@85761a4. This PR can be closed.

Validate add_to_policy inputs

aa430e9

assisted-by-ai added the codex label Dec 2, 2025 — with ChatGPT Codex Connector

ArrayBolt3 reviewed Dec 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate add_to_policy inputs#1

Validate add_to_policy inputs#1
assisted-by-ai wants to merge 1 commit into
masterfrom
codex/find-bugs-in-permission-hardener

assisted-by-ai commented Dec 2, 2025

Uh oh!

assisted-by-ai commented Dec 3, 2025

Uh oh!

ArrayBolt3 Dec 5, 2025

Uh oh!

ArrayBolt3 Dec 5, 2025

Uh oh!

ArrayBolt3 Dec 5, 2025

Uh oh!

ArrayBolt3 Dec 5, 2025

Uh oh!

ArrayBolt3 Dec 5, 2025

Uh oh!

ArrayBolt3 Dec 5, 2025

Uh oh!

ArrayBolt3 Dec 5, 2025

Uh oh!

ArrayBolt3 Dec 5, 2025

Uh oh!

ArrayBolt3 Dec 5, 2025

Uh oh!

ArrayBolt3 Dec 5, 2025

Uh oh!

ArrayBolt3 commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if ! [[ "${did_undo}" = 'false' ]]; then
		if [[ "${did_undo}" = 'false' ]]; then

		echo "... '${file_name_from_stat}' does not exist"
		echo "... '${state_file_item}' does not exist"

Conversation

assisted-by-ai commented Dec 2, 2025

Summary

Testing

Uh oh!

assisted-by-ai commented Dec 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArrayBolt3 commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants