Skip to content

fix: correct regex anchor #310

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

cx-miguel-neiva
Copy link
Contributor

@cx-miguel-neiva cx-miguel-neiva commented Jul 17, 2025

Closes #307

Proposed Changes

  • Fixes an issue where regex patterns ending with $ stopped matching end-of-line in multi-line files, which affected detection of secrets stored on a single line (e.g., in files named PASSWORD or SECRET).
  • Ensures that newline indices are correctly calculated using
    regexp.MustCompile("\n|$").FindAllStringIndex(fragment.Raw, -1),
    so that $ in regex rules continues to match end-of-line as expected.
  • Maintains compatibility with existing detection rules that rely on $ for line endings.
  • Note: To implement this fix, it was necessary to pull the detect logic into our own codebase, allowing us to adjust and maintain the detection behavior as required.

Checklist

  • I covered my changes with tests.
  • I updated the documentation that is affected by my changes:
    • Change in the CLI arguments
    • Change in the configuration file
  • I submit this contribution under the Apache-2.0 license.

@cx-miguel-neiva cx-miguel-neiva requested a review from a team as a code owner July 17, 2025 15:38
Copy link

github-actions bot commented Jul 17, 2025

kics-logo

KICS version: v1.7.13

Category Results
HIGH HIGH 0
MEDIUM MEDIUM 0
LOW LOW 0
INFO INFO 0
TRACE TRACE 0
TOTAL TOTAL 0
Metric Values
Files scanned placeholder 13
Files parsed placeholder 13
Files failed to scan placeholder 0
Total executed queries placeholder 53
Queries failed to execute placeholder 0
Execution time placeholder 1

Copy link

github-actions bot commented Jul 17, 2025

Logo
Checkmarx One – Scan Summary & Details1cb123cb-cdd2-476f-b7f8-501ac7275081

Great job! No new security vulnerabilities introduced in this pull request

@cx-miguel-neiva cx-miguel-neiva force-pushed the fix/regex-last-line-anchor branch from 0fe47da to 4fdbe8b Compare August 4, 2025 11:55
Copy link
Contributor

@cx-rogerio-dalot cx-rogerio-dalot left a comment

Choose a reason for hiding this comment

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

Can we try to slim down the code that we are importing and do our specific logic after detection or before detection on our wrapper?

)

// Detector is the main detector struct
type Detector struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be a wrapper, hold a pointer to the gileaks detector and leave the most of it to the gitleaks detector. Only pull something to here if absolutely necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

The same for the other structs.

Comment on lines +27 to +29
if runtime.GOOS == "windows" {
executable += ".exe"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we avoid this? The tests will run on linux always on CI/CD and locally its nice if you develop making sure that the tests will pass on CI/CD. To fix this for your side, you should just run the tests on WSL.

Comment on lines +368 to +369
- be73c0549f927433e81284de4b3d1094fc3e0e20 # engine/engine_test.go - generic-api-key
- 234b995eeac64bf996c32397906ff8ce1aacbe10 # engine/engine_test.go - jfrog-api-key
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you have some ids repeated (they changed IDs between commits but they refer to the same secret). Can you completely clean your changes in this file and run 2ms so you can add them in one go to avoid cluttering the file unecessarily.

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.

regex option doesn't manage end of line anymore ($)
2 participants