Skip to content

Conversation

@lamhaison
Copy link
Owner

fix bug, remove temp data and improve scan secret for git repo function

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the git-secrets scanning functionality by improving the Docker image build process, adding a comprehensive scan function, fixing documentation bugs, and cleaning up unused code. The changes focus on better security pattern detection, improved user experience with detailed help messages, and code maintainability.

Key Changes

  • Enhanced Docker git-secrets image with additional security patterns (JWT, database connection strings, private keys) and optimized build process
  • Added new lhs_docker_scan_secrets function with multiple scan modes (history, cached, untracked, recursive) and comprehensive help documentation
  • Fixed incorrect history control comments in main.sh and improved cache expiration documentation
  • Removed unused functions and environment variable references to clean up the codebase

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
services/docker.sh Enhanced git-secrets Docker image with more security patterns, added comprehensive scan function with multiple modes and validation checks, and improved user-facing documentation with emoji-prefixed messages
main.sh Fixed incorrect history control comments that previously listed 'ignoredups' multiple times with confusing explanations, added clear cache expiration time documentation
common/peco/peco_helm.sh Changed quote style from double to single quotes around 'NAMESPACE' for consistency
common/peco/peco.sh Removed unused lhs_peco_history function and references to undefined RAKKAR_TF_IAC_PATH environment variable, refactored cache expiration logic for clarity
.gitignore Added test file for secret scanning demonstrations
.gitallowed Created new file with patterns to prevent false positives when scanning git-secrets configuration files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

RUN git secrets --add --global "[a-zA-Z0-9]{32,}"
# Database connection strings
RUN git secrets --add --global "(mongodb|mysql|postgres)://[^\\s]*:[^\\s]*@"
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The double backslashes in the regex pattern may not work as intended in the Dockerfile RUN command. The pattern "(mongodb|mysql|postgres)://[^\\s]:[^\\s]@" uses \s which will be interpreted by the shell. Consider using single backslash or testing to ensure the pattern correctly matches database connection strings with proper escaping.

Suggested change
RUN git secrets --add --global "(mongodb|mysql|postgres)://[^\\s]*:[^\\s]*@"
RUN git secrets --add --global "(mongodb|mysql|postgres)://[^\s]*:[^\s]*@"

Copilot uses AI. Check for mistakes.
local final_projects=$(
cat <<-__EOF__
${LHS_TOOLS_DIR}
cat <<-__EOF__
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

There is trailing whitespace after the heredoc declaration. This should be removed for consistency and to avoid potential issues with some shell parsers.

Suggested change
cat <<-__EOF__
cat <<-__EOF__

Copilot uses AI. Check for mistakes.
# This file prevents false positives when git-secrets scans configuration files

# Allow pattern definitions in configuration files
BEGIN.*PRIVATE.*KEY
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The .gitallowed entry BEGIN.*PRIVATE.*KEY effectively allows all private key blocks to bypass git-secrets detection, so any real private key committed to this repository will not be flagged. This weakens your secret-scanning control and can lead to unnoticed exposure of TLS, SSH, or other private keys to anyone with repository access. Consider removing or drastically narrowing this allow pattern so only the specific non-sensitive example lines are exempted.

Suggested change
BEGIN.*PRIVATE.*KEY

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

echo "📖 Usage: docker run -v \$(pwd):/repository ${image_name} git secrets --scan"
echo "🔍 Scan current directory: docker run -v \$(pwd):/repository ${image_name} git secrets --scan"
echo "🛠️ Install hooks: docker run -v \$(pwd):/repository ${image_name} git secrets --install"
echo "💡 Tip: Run 'lhs_docker_create_gitallowed' to create .gitallowed file for false positives"
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The function 'lhs_docker_create_gitallowed' referenced in this message does not exist in the codebase. This will cause confusion for users when they try to run the suggested command. Either implement this function or update the message to provide the correct guidance (e.g., manually creating a .gitallowed file).

Suggested change
echo "💡 Tip: Run 'lhs_docker_create_gitallowed' to create .gitallowed file for false positives"
echo "💡 Tip: To create a .gitallowed file for false positives, run: touch .gitallowed"

Copilot uses AI. Check for mistakes.
echo ""
echo "💡 Tips:"
echo " • Create .gitallowed file to handle false positives"
echo " • Run lhs_docker_create_gitallowed for common exclusions"
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The function 'lhs_docker_create_gitallowed' referenced in this help message does not exist in the codebase. This will cause confusion for users when they try to run the suggested command. Either implement this function or update the message to provide the correct guidance.

Suggested change
echo "Run lhs_docker_create_gitallowed for common exclusions"
echo "See https://github.com/awslabs/git-secrets#gitallowed-file for common exclusions format"

Copilot uses AI. Check for mistakes.
# Git allow pattern
# RUN git secrets --add --global --allowed '(uuid_1|uuid_2)'
# Generic JWT tokens
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The comment "Generic JWT tokens" could be misleading. JWT tokens themselves are not secrets - they are meant to be transmitted. The secret that needs protection is the JWT signing key, not the token itself. Consider clarifying this comment to explain why JWT patterns are being detected (e.g., "JWT tokens that may contain sensitive claims" or reconsider if this pattern should be included at all).

Suggested change
# Generic JWT tokens
# JWT tokens (may contain sensitive claims or information; not secrets themselves, but their accidental inclusion may expose sensitive data)

Copilot uses AI. Check for mistakes.
Comment on lines +66 to +68
RUN git clone https://github.com/awslabs/git-secrets.git && \
cd git-secrets && \
make install
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

While combining package installation and cleanup in a single RUN command is a Docker best practice for reducing image size, consider splitting the git-secrets installation (lines 66-68) into a separate RUN command. Currently, if git-secrets installation fails, the entire apt layer needs to be rebuilt. Separating these layers would provide better caching during development and debugging.

Suggested change
RUN git clone https://github.com/awslabs/git-secrets.git && \
cd git-secrets && \
make install
RUN git clone https://github.com/awslabs/git-secrets.git
RUN cd git-secrets && make install

Copilot uses AI. Check for mistakes.
- Added .github/workflows/security.yml for automated security checks on push and pull requests.
- Updated .gitallowed to reflect changes in allowed patterns for secret detection.
- Introduced .gitleaksignore to manage false positives in secret scanning.
- Created .pre-commit-config.yaml for local development hooks including shellcheck and gitleaks.
- Added CHANGELOG.md to document notable changes and versioning policy.
- Enhanced README.md with security notices and setup instructions.
- Established SECURITY.md outlining security best practices and reporting vulnerabilities.
- Refactored help_menu.sh and peco scripts for improved functionality and code quality.
- Removed deprecated console_editor.sh, gitlab.sh, and jenkins.sh scripts.
- Updated docker.sh to streamline Docker commands and improve readability.
- Improved git.sh for better handling of Docker volume mounts and command execution.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 21 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

common/utils.sh Outdated
Comment on lines 93 to 95
log_file_path=${aws_cli_logs}/$(hostname).log
else
log_file_path=${aws_cli_logs}/${ASSUME_ROLE}.log
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The variable 'aws_cli_logs' should be quoted in the path assignment to prevent issues with paths containing spaces. This follows shell scripting best practices for robust path handling.

Suggested change
log_file_path=${aws_cli_logs}/$(hostname).log
else
log_file_path=${aws_cli_logs}/${ASSUME_ROLE}.log
log_file_path="${aws_cli_logs}/$(hostname).log"
else
log_file_path="${aws_cli_logs}/${ASSUME_ROLE}.log"

Copilot uses AI. Check for mistakes.
common/utils.sh Outdated
log_file_path=${aws_cli_logs}/${ASSUME_ROLE}.log
fi

tee_command="tee -a ${log_file_path}"
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The variable 'log_file_path' should be quoted in the assignment to prevent issues with paths containing spaces. This ensures the tee command receives a properly quoted path.

Suggested change
tee_command="tee -a ${log_file_path}"
tee_command="tee -a \"${log_file_path}\""

Copilot uses AI. Check for mistakes.

function lhs_docker_build_git_secret_image() {
local image_name="gitsecrets"
docker build -t ${image_name} - <<-EOF
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The variable 'image_name' should be quoted in the docker build command to prevent issues with special characters or spaces. This follows shell script best practices for variable expansion.

Suggested change
docker build -t ${image_name} - <<-EOF
docker build -t "${image_name}" - <<-EOF

Copilot uses AI. Check for mistakes.
exit_code=$?
else
echo "⏳ Scanning files..."
docker run -v "$scan_path:/repository" gitsecrets git secrets --scan $scan_options
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The variable 'scan_options' should be quoted in the docker run command. Without quotes, if 'scan_options' is empty, it will cause the command to fail or behave incorrectly. The variable expansion should be handled properly to avoid word splitting issues.

Suggested change
docker run -v "$scan_path:/repository" gitsecrets git secrets --scan $scan_options
docker run -v "$scan_path:/repository" gitsecrets git secrets --scan "$scan_options"

Copilot uses AI. Check for mistakes.
Comment on lines 6 to 110
## Setup dependencies
Notes: This document is for macos environment.

### Install gitlab cli
### Install GitHub cli
```
brew install gh
export GH_TOKEN=xxxx
```

### Install peco
To allow searching by console.
![image](./images/peco_history_menu.png)

```
brew install glab
export GITLAB_TOKEN=xxxxx
brew install peco
peco --version
peco version v0.5.10 (built with go1.19.2)
```

### Install jq
```
brew install jq
jq --version
jq-1.6
```


## Setup lhs-helpful-commandlines
### Setup from homebrew
**It is easy to setup and run and don't want to change or optimize it**
#### Install
```
brew tap lamhaison/formulae
brew install lamhaison/formulae/lhs-helpful-commandlines
```
## Load when start an Iterm terminal
Add these lines to ~/.bashrc or ~/.zshrc or ~/.bash_profile
```
source "$(which lhs-helpful-commandlines.sh)" "/opt/homebrew/Cellar/lhs-helpful-commandlines/$(brew info lhs-helpful-commandlines | grep -E '==> .*: stable (.*)$' | sed -E 's|==> .*: stable (.*)$|\1|g')" "True" "True"
```

### Re-install the latest version(If there are new versions)
```
brew uninstall lhs-helpful-commandlines
brew untap lamhaison/formulae
brew tap lamhaison/formulae
brew install lamhaison/formulae/lhs-helpful-commandlines
```

## Settings when open terminal (I am using Iterm2)
**It is easy for you to custom your scripting to fix with your style**

```
mkdir -p /opt/lamhaison-tools && cd /opt/lamhaison-tools
git clone https://github.com/lamhaison/helpful-commandlines.git
echo "source /opt/lamhaison-tools/helpful-commandlines/main.sh" >> ~/.bashrc
```

# How to use it?

## How to search helpful commandline
```
Ctrl + h: to and choose the commandline that you want to run(searching and enter to auto fill it to your terminal)
lhs_help_helpful cmd: only for searching, it will not automatically fill in to your terminal
```

## How to search your history commandlines
```
Option + r: to select the history commandline that you wan to re-run(searching and enter to auto fill it to your terminal)
search history commandline: only for searching history, it will not automatically fill in to your termial
```

## How to enable git commit suggestions
```
lhs_git_commit_suggestions: only for searching the list commit message pattern
```

## Security & Contributing

### 🔒 Security Commitment
- ✅ No hardcoded credentials or secrets
- ✅ Automated secret detection on every commit
- ✅ Shell script validation and quality checks
- ✅ Code formatting standards enforcement

**Report security issues**: See [SECURITY.md](SECURITY.md) - Do NOT open public issues for security vulnerabilities.

### CI/CD Security Checks
All pull requests are automatically scanned for:
- 🔐 Secrets and credentials (gitleaks)
- 🐚 Shell script quality (shellcheck)
- 🎨 Code formatting (shfmt)
- ✅ Syntax validation

View the [security workflow](.github/workflows/security.yml) and [changelog](CHANGELOG.md).

## License

This project is licensed under the Apache License 2.0 - see [LICENSE](LICENSE) file for details.

Notes: This document is for macos environment.

### Install GitHub cli
```
brew install gh
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The README contains duplicate content. Lines 106-110 repeat the "Install GitHub cli" section that already appears at lines 9-13. This creates confusion and maintenance burden. The duplicate section should be removed to maintain a single source of truth.

Copilot uses AI. Check for mistakes.

This project is licensed under the Apache License 2.0 - see [LICENSE](LICENSE) file for details.

Notes: This document is for macos environment.
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The correct spelling for Apple's operating system is 'macOS' (not 'macos'). This improves consistency and professionalism.

Suggested change
Notes: This document is for macos environment.
Notes: This document is for macOS environment.

Copilot uses AI. Check for mistakes.
common/utils.sh Outdated
# Replace by using lhs-cli later
function lhs_cmd_file_name_get_random_name() {
local file_name=${1:-'FILENAME'}
echo "${file_name}-$(lhs_cmd_date_get_with_format)"
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

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

The reimplemented 'lhs_cmd_file_name_get_random_name' function no longer generates truly random filenames. The old implementation used 'mktemp' which creates unique temporary files with random suffixes, while the new version uses a timestamp which could result in collisions if called multiple times within the same second. This could lead to file overwrites or conflicts in concurrent scenarios.

Suggested change
echo "${file_name}-$(lhs_cmd_date_get_with_format)"
mktemp --tmpdir "${file_name}.XXXXXX"

Copilot uses AI. Check for mistakes.
- Updated indentation from tabs to spaces in various scripts for better readability.
- Ensured consistent use of spacing around operators and keywords.
- Improved the clarity of comments and echo statements across multiple service scripts.
- Adjusted function definitions and command invocations to follow a uniform style.
- Enhanced the overall maintainability of the codebase by adhering to a consistent coding standard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants