Improved Usernames / Organization / Groups Path Matching in credentials

[macmonac]

User description

The old version only supported host verification. When combined with useHttpPath, it could only check a single-level path for usernames, organizations, or groups, not the full path.
If the first line matched the host or the first-level path, those credentials were returned, even if a later line specified a more precise path.

Example:
https://USERNAME:TOKEN1@gitlab.com/group/subgroup1/project.git
https://USERNAME:TOKEN2@gitlab.com/group/subgroup2/project2.git
https://USERNAME:TOKEN3@gitlab.com/group/subgroup2/
https://USERNAME:TOKEN4@gitlab.com/group/

For the repository:
https://USERNAME:TOKEN2@gitlab.com/group/subgroup2/project2.git

Without useHttpPath: Only the host, protocol, and HTTP username (if specified) are checked. It returns TOKEN1.
With useHttpPath (old version), only the first-level path was checked, even though the rest of the path did not match. In this case, it’s the same as above, so it returns TOKEN1.

Now, The system checks if the path configured in the credentials is the beginning of the requested path.

For the same repository:
https://USERNAME:TOKEN2@gitlab.com/group/subgroup2/project2.git

Without useHttpPath: Behavior remains unchanged (returns TOKEN1).
With useHttpPath (new version): It can now match the full path or a partial path, regardless of the number of username/organization/group levels. Since the full path of the first line doesn’t match the request, it continues to the second line, which does match. It returns TOKEN2.

Notes:
The third line provides global credentials for other projects under /group/subgroup2/.
The fourth line provides global credentials for other projects under /group/.

Important:
The order of lines in the credentials file matters. More specific paths should be listed before broader ones.

I hope this doesn’t alter the originally intended behavior.

If the pull request is accepted, I think we’ll need to update the end of the documentation.

---

PR Type

Enhancement, Bug fix

---

Description

• Improved path matching to support full paths instead of single-level

• Now checks if configured path is prefix of requested path

• Added support for ignoring comments in credentials file

• Simplified path matching logic with strings.HasPrefix

---

Diagram Walkthrough

flowchart LR
  A["Old Path Matching"] -->|"Single-level only"| B["Limited accuracy"]
  C["New Path Matching"] -->|"Full path prefix check"| D["Better accuracy"]
  E["Comment Support"] -->|"Skip # and //"| F["Cleaner config files"]

Loading

File Walkthrough

	Relevant files
Enhancement	main.go Simplify path matching and add comment filtering                  main.go Replaced single-level path matching with strings.HasPrefix for full path comparison Simplified path matching logic by removing strings.Cut and separate handling branches Added comment filtering to skip lines starting with # or // in credentials file Improved logging to show actual paths being compared +10/-13

---

[qodo-free-for-open-source-projects]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🔴	Path prefix matching vulnerability Description: The path matching logic uses strings.HasPrefix which allows any credential with a path prefix to match, potentially exposing credentials to unintended repositories (e.g., credentials for /group/sub would match /group/subgroup or /group/sub-malicious). main.go [103-103] Referred Code match = match && strings.HasPrefix(req.path, c.path) log.Printf("match path by username or org: req.path=%v,config.path=%v,result=%v",
	Credential path information disclosure Description: Sensitive credential paths are logged which may expose repository structure and credential configuration details in log files, potentially aiding attackers in credential enumeration attacks. main.go [104-105] Referred Code log.Printf("match path by username or org: req.path=%v,config.path=%v,result=%v", req.path, c.path, match)
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Credential path exposure: Log statements at lines 104-105 and 229 expose credential file paths and matching logic details that could reveal system configuration to unauthorized users if logs are accessible. Referred Code log.Printf("match path by username or org: req.path=%v,config.path=%v,result=%v", req.path, c.path, match) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Prefix matching edge case: The new strings.HasPrefix logic at line 103 may incorrectly match credentials when c.path is empty, potentially returning overly broad credentials without validation. Referred Code match = match && strings.HasPrefix(req.path, c.path) log.Printf("match path by username or org: req.path=%v,config.path=%v,result=%v", Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Potential credential exposure: Log statements at lines 104-105 and 229 may inadvertently log sensitive path information that could contain usernames or organizational structure, requiring verification that no sensitive data is exposed. Referred Code log.Printf("match path by username or org: req.path=%v,config.path=%v,result=%v", req.path, c.path, match) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Path validation missing: The prefix matching at line 103 lacks validation to ensure c.path and req.path are properly sanitized, which could lead to unintended credential matching if paths contain special characters or path traversal sequences. Referred Code match = match && strings.HasPrefix(req.path, c.path) log.Printf("match path by username or org: req.path=%v,config.path=%v,result=%v", Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-free-for-open-source-projects]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix incorrect partial path matching To prevent incorrect partial path matches (e.g., org matching org-backup), modify the logic to check for either an exact match or a prefix followed by a /. main.go [103] - match = match && strings.HasPrefix(req.path, c.path) + // A path from credentials matches if it's an exact match, or if it's a prefix of the request path ending with a '/' + match = match && (req.path == c.path || strings.HasPrefix(req.path, c.path+"/")) Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant logic flaw introduced in the PR, where partial path prefixes could be incorrectly matched, potentially leading to the use of wrong credentials.	High
General	Avoid redundant string trimming operations Avoid calling strings.TrimSpace(line) twice by storing its result in a variable before checking for comment prefixes. main.go [228-231] - if strings.HasPrefix(strings.TrimSpace(line), "#") || strings.HasPrefix(strings.TrimSpace(line), "//") { + trimmedLine := strings.TrimSpace(line) + if strings.HasPrefix(trimmedLine, "#") || strings.HasPrefix(trimmedLine, "//") { log.Printf("ignore comment : %s", line) continue } Apply / Chat Suggestion importance[1-10]: 3 __ Why: The suggestion offers a minor optimization by avoiding a redundant function call, which improves code cleanliness and efficiency slightly, but the performance impact is negligible.	Low
No more code suggestions

[ttys3]

Claude Sonnet 4.5:

I'll review this PR by first examining the original code and then the patch changes.Now let me review this PR according to Git's official documentation on credential storage.

PR Review: git-credential-readonly Pull Request #4

This PR contains two commits that make changes to the credential matching logic. Let me analyze each according to Git's credential storage documentation.

Commit 1: Ignore comments in credentials

Changes: Adds support for comment lines (starting with # or //) in the credentials file.

Analysis:
According to Git's official documentation, "No other kinds of lines (e.g. empty lines or comment lines) are allowed in the file" in the .git-credentials format. The standard git-credential-store explicitly does not support comments.

Verdict: ❌ This violates the Git credential storage format specification. While the feature might be convenient, it makes the credential file incompatible with the standard git-credential-store helper. If users alternate between this helper and the standard one, they may encounter issues.

Recommendation: If you want to add this feature, document clearly that files with comments will not work with standard git-credential-store and recommend users maintain separate credential files.

---

Commit 2: Improved path matching

Changes: Simplifies path matching from checking only the first-level path component to using strings.HasPrefix() to match any path prefix.

Analysis:

The original code had complex logic to extract and match only the first path component (username/org). The new code uses prefix matching, which means:

• https://user:token@gitlab.com/group/ would match requests for https://user:token@gitlab.com/group/subgroup/project.git
• More specific credentials should be listed first in the file

According to Git's documentation, when using useHttpPath, Git considers credentials to match "if the context is a more-specific subset of the pattern in the config file". However, the standard git-credential-store matches "if the protocol, hostname, and username (if we already have one) match" - it doesn't do partial path matching in the way this PR implements.

Issues identified:

1. Security concern: The prefix matching could be too permissive. For example:

   • Credential: https://user:token@example.com/my
   • Would match: https://user@example.com/myproject (intended)
   • Would also match: https://user@example.com/my-private-repo (possibly unintended)

2. Order dependency: The PR correctly notes that "more specific paths should be listed before broader ones," but this creates a maintenance burden and potential security issue if the file is reordered.

3. Inconsistent with standard behavior: The standard git-credential-store doesn't do prefix matching on paths - it requires exact matches when paths are involved.

Verdict: ⚠️ Partially problematic. While the feature adds useful functionality for hierarchical path structures (like GitLab groups), the implementation using simple prefix matching is potentially unsafe and inconsistent with Git's standard credential matching behavior.

Recommendations:

1. For security: Add path separator awareness to prevent partial matches:

   configPath := strings.TrimRight(c.path, "/")
   requestPath := strings.TrimRight(req.path, "/")
   match = match && (requestPath == configPath || strings.HasPrefix(requestPath, configPath + "/"))

2. Consider exact matching mode: Add a flag or format indicator to specify whether a credential should use prefix matching or require exact path match.

3. Document the behavior clearly: Users need to understand that this deviates from standard Git behavior and the security implications.

---

Overall Recommendation

Do not merge as-is. Both commits introduce behavior that deviates from Git's standard credential storage format and matching logic in ways that could cause compatibility issues or security concerns. Consider:

1. Making comment support optional and clearly documented
2. Improving the path matching logic to be safer (path-separator-aware)
3. Documenting all deviations from standard Git behavior
4. Consider whether these features belong in a "read-only" credential helper or if they should be in a separate, more feature-rich helper with a different name