Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tree: run shellharden on bash files #328

Closed

Conversation

ethancedwards8
Copy link
Member

@ethancedwards8 ethancedwards8 commented Feb 16, 2025

Running shellharden on bash files makes them more secure and less buggy as vulnerabilities to malicious glob expansion and the like are reduced.

GHA CI check for PRs incoming... eventually...

Running shell harden on bash files makes them more secure and less buggy
as vulnerabilities to malicious glob expansion and the like are reduced.

GHA CI check for PRs incoming...

Signed-off-by: Ethan Carter Edwards <[email protected]>
@ethancedwards8
Copy link
Member Author

@Theoreticallyhugo I would appreciate it you would look over this/test it to see if there is anything I overlooked

@Theoreticallyhugo
Copy link
Collaborator

will do in the next two-three days!

@@ -130,13 +130,13 @@ main() {
true)
flags="#{?window_flags,#[fg=${dark_purple}]#{window_flags},}"
current_flags="#{?window_flags,#[fg=${light_purple}]#{window_flags},}"
esac
;; esac
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i find it somewhat confusing that the ;; is at the beginning of this and not the end of the previous line, but thats nitpicking on my part. no need to change it if you dont wanna

@Theoreticallyhugo
Copy link
Collaborator

Theoreticallyhugo commented Feb 19, 2025

the ssh-session plugin broke, and i havent yet found out why.

ssh_user=$(whoami)
fi

echo $ssh_user
echo "$ssh_user"
}

get_remote_info() {
local command=$1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this variable appears to be unused?

fi
}

ssh_connected() {
# Get current pane command
local cmd=$(tmux display-message -p "#{pane_current_command}")

[ $cmd = "ssh" ] || [ $cmd = "sshpass" ]
[ "$cmd" = "ssh" ] || [ "$cmd" = "sshpass" ]
Copy link
Collaborator

@Theoreticallyhugo Theoreticallyhugo Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason the entire script only works if this line is replaced by the following:

  if [ "$cmd" = "ssh" ] || [ "$cmd" = "sshpass" ]; then
    echo true
  else
    echo false
  fi

i dont really get why.
tested on macos sequoia 15.2. (tested it thoroughly, rebooted my mac and tested again)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(will probably test on linux sometime later today or tomorrow)

@@ -43,7 +43,7 @@ main()
getFullMessage
fi

sleep $RATE
sleep "$RATE"
}

getFullMessage()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the following lines ! -z is used, and since shellharden replaces all -n and -z, we might want to replace these too.

output=$(echo "$branch")
else
output=$(echo "$diff_symbol $branch")
fi
fi

else
if [ $(checkEmptySymbol $current_symbol) == "true" ]; then
if [ "$(checkEmptySymbol "$current_symbol")" == "true" ]; then
output=$(echo "$branch")
else
output=$(echo "$current_symbol $branch")
Copy link
Collaborator

@Theoreticallyhugo Theoreticallyhugo Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a reduntant echo isnt it? (and many others in this file)

output=$(echo "${changes} $branch")
else
output=$(echo "$diff_symbol ${changes} $branch")
fi
else
if [ $(checkEmptySymbol $diff_symbol) == "true" ]; then
if [ "$(checkEmptySymbol "$diff_symbol")" == "true" ]; then
output=$(echo "$branch")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there seem to be more reduntant echos in this file.

@@ -18,18 +18,18 @@ getChanges()
declare -i updated=0;
declare -i deleted=0;

for i in $(git -C $path --no-optional-locks status -s)
for i in "$(git -C "$path" --no-optional-locks status -s)"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these double quotes seem to break the program

}


# getting the #{pane_current_path} from dracula.sh is no longer possible
getPaneDir()
{
nextone="false"
for i in $(tmux list-panes -F "#{pane_active} #{pane_current_path}");
for i in "$(tmux list-panes -F "#{pane_active} #{pane_current_path}")";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these double quotes seem to break the program

}


# getting the #{pane_current_path} from dracula.sh is no longer possible
getPaneDir()
{
nextone="false"
for i in $(tmux list-panes -F "#{pane_active} #{pane_current_path}");
for i in "$(tmux list-panes -F "#{pane_active} #{pane_current_path}")";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i cant test fossil, but this seems to be the same issue as with git

@@ -18,18 +18,18 @@ getChanges()
declare -i updated=0;
declare -i deleted=0;

for i in $(cd $path; fossil changes --differ|cut -f1 -d' ')
for i in "$(cd "$path"; fossil changes --differ|cut -f1 -d' ')"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i cant test fossil, but this seems to be the same issue as with git

@@ -7,7 +7,7 @@ source "$current_dir/utils.sh"
getPaneDir() {
nextone="false"
ret=""
for i in $(tmux list-panes -F "#{pane_active} #{pane_current_path}"); do
for i in "$(tmux list-panes -F "#{pane_active} #{pane_current_path}")"; do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to break the scripts functionality

@ethancedwards8
Copy link
Member Author

@Theoreticallyhugo i think it would just be best if I close this PR. I was a bit naive to do this, lol.

@Theoreticallyhugo
Copy link
Collaborator

yeah no makes sense 😅

@ethancedwards8
Copy link
Member Author

Thanks for all your work on testing it. I should have been more diligent.

@ethancedwards8 ethancedwards8 deleted the apply-shellcheck branch February 20, 2025 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants