os-git-backup: SCP-style SSH URLs corrupted by credential injection
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 1.2k
- Forks
- 863
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 10
Description
Important notices
Before you add a new report, we ask you kindly to acknowledge the following:
- I have read the contributing guide lines at https://github.com/opnsense/plugins/blob/master/CONTRIBUTING.md
- I have searched the existing issues, open and closed, and I'm convinced that mine is new.
- The title contains the plugin to which this issue belongs
Describe the bug
backup() in src/opnsense/mvc/app/library/OPNsense/Backup/Git.php corrupts SCP-style SSH URLs (git@github.com:user/repo.git) when the User Name field is empty. strpos($url, '//') returns false for URLs without //, PHP evaluates false + 2 as 2, and substr() splits the URL at position 2:
git@github.com:user/repo.git
-> gi + @ + t@github.com:user/repo.git
-> gi@t@github.com:user/repo.git
git push then fails with an authentication error against the invalid remote URL.
Two defects combine here (lines 165-172):
- The code uses
$posinsubstr()without checking forfalse. SCP-style URLs contain no//. - For non-HTTP URLs the code injects the user even when the field is empty, producing a bare
@.
Last known working version: none known.
To Reproduce
- Install os-git-backup
- Set URL to
git@github.com:user/repo.git - Leave User Name empty
- Provide an SSH private key
- Run the backup
Expected behavior
The SCP-style URL passes through to git remote add origin unchanged; the SSH key handles authentication.
Screenshots
Not applicable.
Relevant log files
The backup log shows git push failing with an authentication error against the corrupted remote URL.
Additional context
Suggested fix: wrap credential injection in a $pos !== false check so the code never modifies SCP-style URLs, and skip injection for SSH URLs without a configured user:
if ($pos !== false) {
if (stripos(trim((string)$mdl->url), 'http') === 0) {
$cred = urlencode((string)$mdl->user) . ":" . urlencode((string)$mdl->password);
$url = substr($url, 0, $pos + 2) . "{$cred}@" . substr($url, $pos + 2);
} elseif (!empty((string)$mdl->user)) {
$url = substr($url, 0, $pos + 2) . urlencode((string)$mdl->user) . "@" . substr($url, $pos + 2);
}
}
Workaround: use ssh://github.com/user/repo.git with git in the User Name field; the plugin then builds ssh://git@github.com/user/repo.git.
Refiled from #5234 on the issue template; closing the original.
Environment
OPNsense 26.1.2
os-git-backup (current plugins master)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Open src/opnsense/mvc/app/library/OPNsense/Backup/Git.php and inspect backup() around lines 165-172, focusing on how strpos() and the empty User Name field affect credential injection. Reproduce with git@github.com:user/repo.git and no User Name, then verify that the remote URL passes unchanged and git push no longer uses the corrupted URL.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, php
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 82/100