Skip to content

Conversation

@ParagEkbote
Copy link
Contributor

@ParagEkbote ParagEkbote commented Oct 22, 2025

Description

Currently, the pre-commit hook for trufflehog fails with a package not installed warning. Since it is a golang package, I have adjusted the configuration for it to work. It currently scans src, tests and .github/workflows in order for the checks to be completed in a reasonable amount of time.

I've excluded .venv from it since it can give false positives as well. Additionally, I've excluded .md and .svg files from being linted every time pre-commit is executed and have linted the pending files as well.

Could you please review?

cc: @johannaSommer

Related Issue

Fixes #(issue number)

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Note

Enables TruffleHog in pre-commit with proper config, upgrades lint hooks, adds md/svg exclusions, and applies minor docs/notebook and code formatting tweaks.

  • CI / Pre-commit:
    • Add TruffleHog Go hook scanning src, tests, .github/workflows with excludes for .venv, .pyc, *.md, *.svg; stages pre-commit and pre-push.
    • Upgrade pre-commit-hooks to v6.0.0 and ruff-pre-commit to v0.14.1.
    • Add consistent excludes for *.md and *.svg across hooks.
    • Refine local hooks: add excludes to ty and check-pruna-pro; remove old shell-based TruffleHog hook.
  • Repo config:
    • Fix .gitignore entry for tests/openai.
  • Docs / Tutorials:
    • Minor RST newline/whitespace fixes and reformat llm_quantization_compilation_acceleration.ipynb JSON.
  • Code style:
    • Small formatting/cleanup in docs/utils/gen_docs.py, src/pruna/data/datasets/text_generation.py, and metrics modules (no functional changes).

Written by Cursor Bugbot for commit 15dedd9. This will update automatically on new commits. Configure here.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Member

@davidberenstein1957 davidberenstein1957 left a comment

Choose a reason for hiding this comment

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

Hi @ParagEkbote , could you disable your automatic formatting for all of the files within the repo? It is generally best practice to have everything nicely formatted according to the standard, but doing it for files you have not touched introduces a bit of uncertainty and bloat to PRs. I believe VS Code has some config for this :)

/resources/llama.cpp/

tests/openai
tests/openai

Choose a reason for hiding this comment

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

why are we exluding these tests? I think we should include them, could you double check if this wasn't a mistake?

Copy link
Contributor Author

@ParagEkbote ParagEkbote Oct 23, 2025

Choose a reason for hiding this comment

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

I have not included this change. It has been there for quite some time, the pre-commit hook must have formatted something 😄 : https://github.com/PrunaAI/pruna/blame/main/.gitignore

.*\.svg$
)
- repo: https://github.com/astral-sh/ruff-pre-commit

Choose a reason for hiding this comment

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

perhaps we could only include certain files? ".py" etc. this would simplify it a bit.

.*\.svg$
)
- repo: https://github.com/trufflesecurity/trufflehog

Choose a reason for hiding this comment

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

Could you rebase on main, I've also redone this implementation and forced only checking for files that have been commited, so this should resolve a huge part of it too. Perhaps we can also raise a certain error if this fails and redirect users to the install page?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, trufflehog is a golang package, not a python package. I think that this approach should simplify the usage for contributors and maintainers and not worry about installation at all. I could reduce the scope of files being checked to further improve the hook. WDYT?

@ParagEkbote
Copy link
Contributor Author

Hi @ParagEkbote , could you disable your automatic formatting for all of the files within the repo? It is generally best practice to have everything nicely formatted according to the standard, but doing it for files you have not touched introduces a bit of uncertainty and bloat to PRs. I believe VS Code has some config for this :)

I have performed the formatting as per the existing pre-commit rules. This is a one-time lint backlog that needs to be cleared otherwise, it is an issue when contributing to the repo, we have to choose our changed files to commit in order to prevent accidental additions. WDYT?

cc: @davidberenstein1957

@davidberenstein1957
Copy link
Member

Hi @ParagEkbote , could you disable your automatic formatting for all of the files within the repo? It is generally best practice to have everything nicely formatted according to the standard, but doing it for files you have not touched introduces a bit of uncertainty and bloat to PRs. I believe VS Code has some config for this :)

I have performed the formatting as per the existing pre-commit rules. This is a one-time lint backlog that needs to be cleared otherwise, it is an issue when contributing to the repo, we have to choose our changed files to commit in order to prevent accidental additions. WDYT?

cc: @davidberenstein1957

I think for now, you can leave it :) However I think it is best practise to avoid changing the formatting of files you have not functionally touched so it would be good to change or verify your setting if it is doing this automatically in a way.

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Pre-commit Hook Bash Syntax Error

The check-pruna-pro pre-commit hook has a bash syntax error. The conditional ["$file" != ".pre-commit-config.yaml"] is missing a space after the opening bracket, causing the hook to fail.

Fix in Cursor Fix in Web

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: TruffleHog Hook Fails with Multiple Paths

The TruffleHog hook configuration passes multiple directory paths ("src", "tests", ".github/workflows") as separate arguments to the "trufflehog filesystem" command. However, the trufflehog filesystem command expects only a single path argument. This will cause the hook to fail during execution because trufflehog will interpret "tests" and ".github/workflows" as invalid arguments rather than additional directories to scan.

Fix in Cursor Fix in Web

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Trufflehog Hook Limits Secret Scanning Scope

The trufflehog hook now only scans hardcoded directories (src, tests, .github/workflows) instead of all staged files. This creates a security gap where secrets in files outside these directories (e.g., docs/, scripts/, root-level configuration files) will not be detected by the secret scanner during pre-commit.

Fix in Cursor Fix in Web

@ParagEkbote
Copy link
Contributor Author

Let me open a separate PR since the diff here is quite large; it will only include the pre-commit file based on your changes. If you have any more comments, feel free to include them in this PR.

cc: @davidberenstein1957

@github-actions
Copy link

github-actions bot commented Nov 3, 2025

This PR has been inactive for 10 days and is now marked as stale.

@github-actions github-actions bot added the stale label Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants