diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 0000000..7512882 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,65 @@ +name: Bug report +description: Something did not work as expected +title: "[Bug]: " +labels: ["bug"] +body: + - type: markdown + attributes: + value: | + Thanks for reporting a bug. Please check the [README](https://github.com/coldsofttech/CommitRocket#readme) + and existing issues first. Do not include secrets, tokens or private file names. + - type: textarea + id: what-happened + attributes: + label: What happened? + description: What did you expect, and what happened instead? + validations: + required: true + - type: textarea + id: command + attributes: + label: Command you ran + description: The full `commitrocket` command, including options. + render: shell + validations: + required: true + - type: textarea + id: output + attributes: + label: Output or error + description: Paste the terminal output. The log file path is printed at the end of every run. + render: text + - type: textarea + id: log + attributes: + label: Relevant log lines + description: Re-run with `--level DEBUG` and paste the relevant part of the log file, with any private paths removed. + render: text + - type: input + id: version + attributes: + label: CommitRocket version + description: Output of `commitrocket --version` + validations: + required: true + - type: dropdown + id: os + attributes: + label: Operating system + options: + - Windows + - macOS + - Linux + - Other + validations: + required: true + - type: input + id: tooling + attributes: + label: Python, Git and git-lfs versions + description: For example `Python 3.12, git 2.47, git-lfs 3.6`. `commitrocket doctor` shows these. + - type: input + id: host + attributes: + label: Git host + description: For example GitHub, GitLab, Bitbucket, self-hosted. diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000..834236c --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,8 @@ +blank_issues_enabled: false +contact_links: + - name: Report a security vulnerability + url: https://github.com/coldsofttech/CommitRocket/security/advisories/new + about: Please report security problems privately, not in a public issue. + - name: Questions and ideas + url: https://github.com/coldsofttech/CommitRocket/discussions + about: Ask a question or float an idea before opening an issue. diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml new file mode 100644 index 0000000..b62a8ea --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -0,0 +1,30 @@ +name: Feature request +description: Suggest an improvement or a new option +title: "[Feature]: " +labels: ["enhancement"] +body: + - type: textarea + id: problem + attributes: + label: What problem are you trying to solve? + description: Describe the situation, not only the solution. + validations: + required: true + - type: textarea + id: proposal + attributes: + label: What would you like to happen? + description: For example a new option, a change in behavior, or a new output. + validations: + required: true + - type: textarea + id: alternatives + attributes: + label: Alternatives you considered + description: Workarounds you use today, or other designs. + - type: checkboxes + id: contribute + attributes: + label: Contributing + options: + - label: I would be willing to open a pull request for this. diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..5fc1d01 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,17 @@ +## What does this change? + + + +## How was it tested? + + + +## Checklist + +- [ ] `uv run ruff check .` passes +- [ ] `uv run pytest -m "not integration"` passes +- [ ] `uv run pytest -m integration` passes (if the change touches the run flow, Git handling or LFS) +- [ ] Tests added or updated for the change +- [ ] `CHANGELOG.md` has a short entry under `[Unreleased]` (what changed for the reader, in one or two lines) +- [ ] `README.md` updated if user-facing behavior or options changed; technical details go in `docs/` +- [ ] No secrets, tokens or private paths in the code, tests or logs diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..2291ec2 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,33 @@ +version: 2 + +updates: + # Python dependencies and the uv.lock file + - package-ecosystem: "uv" + directory: "/" + schedule: + interval: "weekly" + commit-message: + prefix: "chore(deps)" + groups: + python-dependencies: + patterns: ["*"] + update-types: ["minor", "patch"] + + # Actions used in .github/workflows + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + commit-message: + prefix: "chore(ci)" + groups: + github-actions: + patterns: ["*"] + + # Hook versions in .pre-commit-config.yaml + - package-ecosystem: "pre-commit" + directory: "/" + schedule: + interval: "weekly" + commit-message: + prefix: "chore(deps)" diff --git a/CHANGELOG.md b/CHANGELOG.md index 716ac5d..038327c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,23 @@ ## [Unreleased] +## [0.1.1] - 2026-10-24 + +### Added +- Issue and pull request templates, a security policy, a code of conduct and contributing guidelines. +- Dependabot keeps dependencies, GitHub Actions and pre-commit hooks up to date. + +### Fixed +- Batches with thousands of files no longer fail on Windows with "The filename or extension is too long". + +### Security +- A config file found inside a repository can no longer turn on `force`, set the remote or branch, write reports or logs, or read pattern files; use `--config` for those. +- File names with characters like `[` or `*` are treated literally, so they no longer pull other files into a batch. +- `--remote` and `--branch` values that look like Git options are rejected. +- Credentials inside remote URLs are hidden in logs, errors and reports, and log and report files are readable only by you. +- File names with line breaks are refused when creating LFS rules. +- Square brackets in file names and Git messages no longer break or hide output. + ## [0.1.0] - 2026-10-24 ### Added diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..c12b40f --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,131 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our +community a harassment-free experience for everyone, regardless of age, body +size, visible or invisible disability, ethnicity, sex characteristics, gender +identity and expression, level of experience, education, socio-economic status, +nationality, personal appearance, race, caste, color, religion, or sexual +identity and orientation. + +We pledge to act and interact in ways that contribute to an open, welcoming, +diverse, inclusive, and healthy community. + +## Our Standards + +Examples of behavior that contributes to a positive environment for our +community include: + +* Demonstrating empathy and kindness toward other people +* Being respectful of differing opinions, viewpoints, and experiences +* Giving and gracefully accepting constructive feedback +* Accepting responsibility and apologizing to those affected by our mistakes, + and learning from the experience +* Focusing on what is best not just for us as individuals, but for the overall + community + +Examples of unacceptable behavior include: + +* The use of sexualized language or imagery, and sexual attention or advances of + any kind +* Trolling, insulting or derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or email address, + without their explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Enforcement Responsibilities + +Community leaders are responsible for clarifying and enforcing our standards of +acceptable behavior and will take appropriate and fair corrective action in +response to any behavior that they deem inappropriate, threatening, offensive, +or harmful. + +Community leaders have the right and responsibility to remove, edit, or reject +comments, commits, code, wiki edits, issues, and other contributions that are +not aligned to this Code of Conduct, and will communicate reasons for moderation +decisions when appropriate. + +## Scope + +This Code of Conduct applies within all community spaces, and also applies when +an individual is officially representing the community in public spaces. +Examples of representing our community include using an official email address, +posting via an official social media account, or acting as an appointed +representative at an online or offline event. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported to the community leaders responsible for enforcement privately through +GitHub, at + +(choose a title that makes clear it is a conduct report). All complaints will be +reviewed and investigated promptly and fairly. + +All community leaders are obligated to respect the privacy and security of the +reporter of any incident. + +## Enforcement Guidelines + +Community leaders will follow these Community Impact Guidelines in determining +the consequences for any action they deem in violation of this Code of Conduct: + +### 1. Correction + +**Community Impact**: Use of inappropriate language or other behavior deemed +unprofessional or unwelcome in the community. + +**Consequence**: A private, written warning from community leaders, providing +clarity around the nature of the violation and an explanation of why the +behavior was inappropriate. A public apology may be requested. + +### 2. Warning + +**Community Impact**: A violation through a single incident or series of +actions. + +**Consequence**: A warning with consequences for continued behavior. No +interaction with the people involved, including unsolicited interaction with +those enforcing the Code of Conduct, for a specified period of time. This +includes avoiding interactions in community spaces as well as external channels +like social media. Violating these terms may lead to a temporary or permanent +ban. + +### 3. Temporary Ban + +**Community Impact**: A serious violation of community standards, including +sustained inappropriate behavior. + +**Consequence**: A temporary ban from any sort of interaction or public +communication with the community for a specified period of time. No public or +private interaction with the people involved, including unsolicited interaction +with those enforcing the Code of Conduct, is allowed during this period. +Violating these terms may lead to a permanent ban. + +### 4. Permanent Ban + +**Community Impact**: Demonstrating a pattern of violation of community +standards, including sustained inappropriate behavior, harassment of an +individual, or aggression toward or disparagement of classes of individuals. + +**Consequence**: A permanent ban from any sort of public interaction within the +community. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], +version 2.1, available at +. + +Community Impact Guidelines were inspired by +[Mozilla's code of conduct enforcement ladder][Mozilla CoC]. + +For answers to common questions about this code of conduct, see the FAQ at +. Translations are available at +. + +[homepage]: https://www.contributor-covenant.org +[Mozilla CoC]: https://github.com/mozilla/diversity diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..89a7d9d --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,62 @@ +# Contributing to CommitRocket + +Thanks for helping out! Bug reports, ideas, documentation fixes and code are all welcome. +By taking part you agree to follow the [Code of Conduct](CODE_OF_CONDUCT.md). + +## Reporting bugs and suggesting features + +- Search the existing issues first. +- Use the issue templates. For a bug, include the exact command, the output, your + versions (`commitrocket --version`, `commitrocket doctor`), and the relevant lines from + the log file (re-run with `--level DEBUG`). Remove secrets and private paths. +- **Security problems:** do not open an issue. Follow the [Security Policy](SECURITY.md). + +## Setting up + +You need Python 3.11+, [Git](https://git-scm.com/) and [uv](https://docs.astral.sh/uv/). + +``` +git clone https://github.com/coldsofttech/CommitRocket.git +cd CommitRocket +uv sync +uv run pre-commit install +``` + +`uv run commitrocket --help` runs the tool from your checkout. + +## Making a change + +1. Create a branch from `main`, for example `feat/short-name`, `fix/short-name` or + `chore/short-name`. +2. Make the change, with tests. New behavior needs a test; a bug fix needs a test that + fails without the fix. +3. Run the checks: + + ``` + uv run ruff check . + uv run pytest -m "not integration" + uv run pytest -m integration + ``` + + Pre-commit runs the first two on every commit. The integration tests drive the real + CLI against a local bare repository and run in CI, so run them yourself when you touch + the run flow, Git handling or LFS. Details are in [docs/development.md](docs/development.md). +4. Update the docs: + - `README.md` is for users: describe options and behavior from a user's point of view. + - Technical notes belong in `docs/`. + - Add a short entry under `[Unreleased]` in `CHANGELOG.md`: one or two lines saying + what changed for the reader, without file or function names. +5. Open a pull request against `main` and fill in the template. Keep it focused; separate + unrelated changes into separate pull requests. + +## Tests that touch Git + +Tests create temporary repositories and never touch your real ones. Do not point a test at +a real remote. Keep any log or report files a test writes outside the repository the test +is operating on. + +## Releases + +Maintainers release by bumping the version in `pyproject.toml` and +`src/commitrocket/__init__.py`, moving the changelog entries under the new version, and +pushing a `v*.*.*` tag. See [docs/development.md](docs/development.md). diff --git a/README.md b/README.md index 6fc5396..94c8250 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,58 @@ # CommitRocket +[![Tests](https://github.com/coldsofttech/CommitRocket/actions/workflows/tests.yml/badge.svg)](https://github.com/coldsofttech/CommitRocket/actions/workflows/tests.yml) +[![Lint](https://github.com/coldsofttech/CommitRocket/actions/workflows/lint.yml/badge.svg)](https://github.com/coldsofttech/CommitRocket/actions/workflows/lint.yml) +[![Release](https://img.shields.io/github/v/release/coldsofttech/CommitRocket?logo=github)](https://github.com/coldsofttech/CommitRocket/releases) +[![PyPI](https://img.shields.io/pypi/v/commitrocket?logo=pypi&logoColor=white)](https://pypi.org/project/commitrocket/) +[![Python 3.11+](https://img.shields.io/badge/python-3.11%2B-blue?logo=python&logoColor=white)](https://www.python.org/downloads/) +[![License](https://img.shields.io/github/license/coldsofttech/CommitRocket)](LICENSE) +[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff) +[![pre-commit](https://img.shields.io/badge/pre--commit-enabled-brightgreen?logo=pre-commit)](https://github.com/pre-commit/pre-commit) + Got a huge pile of changes that a single push would choke on? CommitRocket splits them into size-limited batches, commits each batch, and pushes as it goes, with live progress. +## Why CommitRocket? + +Pushing a lot of data in one go tends to go wrong. A single commit with gigabytes of videos, +datasets, archives or generated assets can time out halfway, be rejected for containing a +file over your host's limit (GitHub refuses anything over 100 MiB), or leave you with nothing +pushed after hours of waiting. CommitRocket turns that one fragile push into many small ones: +each batch is committed and uploaded on its own, so progress is saved as you go, a failure +costs you one batch instead of everything, and you can always see how far along you are. + +It is a good fit for: + +- **First uploads of big folders**: course material, media libraries, backups, game assets, + data dumps, or a repository you are migrating. +- **Slow or unreliable connections**, where small pushes that can be retried beat one huge one. +- **Repositories with a few very large files**, which it can store with Git LFS for you + without leaving LFS configuration behind. +- **Unattended runs**: scheduled jobs and CI, with a JSON report and notifications. + +## What you get + +- **Size-based batching**: choose how big each commit is, how files are picked (in order, or + packed tightly), and the largest single file you will accept. Anything too large is skipped + and listed at the end instead of failing the run. +- **Live progress** with elapsed time and an ETA based on how fast your upload really is. +- **Safe with large files**: files over the 100 MiB host limit are detected before anything + is committed. With `--use-lfs` they go through Git LFS, installed and removed again for you. +- **Recovers from trouble**: failed pushes are retried, every push is checked against the + remote, and if a run is interrupted (even with Ctrl+C) you just run the same command to + carry on from where it stopped. +- **Under your control**: preview everything with `--dry-run`, pick files with + `.gitignore`-style patterns, choose the remote and branch, commit without pushing, and + shape the commit messages with a template. +- **Easy to automate**: save your options in a config file, read results from a JSON + report, get a desktop or webhook notification when a long run ends, and check your setup + with `commitrocket doctor`. +- **Leaves a trail**: every run writes a log file you can attach to a bug report. + +It runs on Windows, macOS and Linux and talks to your remote through plain Git, so it works +with any host you can already push to. It never changes your global Git configuration: +temporary settings live in the repository and are restored afterwards. + ## Install ``` @@ -106,6 +156,14 @@ message = "Videos" retries = 5 ``` +**A config file inside a repository is not fully trusted**, because anyone who can change the +repository can change it. If you run CommitRocket in a repository you cloned, a file found +there is not allowed to turn on `force`, choose `remote` or `branch`, set `report`, +`log-file` or `notify-webhook`, or read pattern files (`include-from`, `exclude-from`). +Those settings are ignored with a warning. To use them, keep them in a file you trust and +pass it explicitly: `commitrocket --config path/to/commitrocket.toml`. They also work as +normal command-line options. + ### Commit message templates `--message-template` decides the message of each batch. The default is `{message} #{n}`. @@ -394,6 +452,12 @@ of the run and are put back afterwards. Your global Git configuration is never t If you run CommitRocket without `--use-lfs` and a file is over 100 MiB, it stops before committing anything and tells you to use the flag. +### Contributing and security + +Bug reports and ideas are welcome: see [CONTRIBUTING.md](CONTRIBUTING.md). Please report +security problems privately, as described in [SECURITY.md](SECURITY.md). Everyone taking +part is expected to follow the [Code of Conduct](CODE_OF_CONDUCT.md). + ### Interrupted or failed runs If a run stops, whether it failed or you pressed Ctrl+C, just run the same command again. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..83c1d7b --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,43 @@ +# Security Policy + +## Supported versions + +Security fixes are made for the latest released version. Please upgrade before reporting +a problem that may already be fixed. + +| Version | Supported | +| --- | --- | +| Latest release | Yes | +| Older releases | No | + +## Reporting a vulnerability + +Please **do not open a public issue** for a security problem. + +Report it privately through GitHub: + +1. Open the repository's **Security** tab. +2. Choose **Report a vulnerability**, or go directly to + . + +Include what you found, the steps to reproduce it, the CommitRocket, Git and git-lfs +versions, your operating system, and what you think the impact is. Please leave out real +credentials and private repository contents. + +You can expect an acknowledgement within about 7 days. After that, we will keep you +updated on the investigation and agree on a date for the fix and its disclosure with you. +Credit is given in the release notes if you would like it. + +## What is in scope + +CommitRocket runs Git commands on your repository and can send an optional webhook +notification, so reports about the following are especially welcome: + +- Commands or paths from repository content, config files or options being run or + interpreted unsafely. +- Files being committed, pushed or written outside what the options and filters allow. +- Secrets, such as webhook addresses, appearing in reports or logs. +- Unsafe handling of the config file, the report file or the log file. + +Problems in Git, Git LFS, your Git host or other dependencies should be reported to those +projects. diff --git a/docs/architecture.md b/docs/architecture.md index ca99e0b..dbab27e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -113,6 +113,33 @@ PowerShell's own AppUserModelID, which Windows already trusts), `osascript` on m restricted to http(s) URLs with a 10 s timeout. Delivery errors are logged and printed, never raised. +## Security hardening + +Findings from the security review and what addresses them (`security.py` holds the helpers): + +- **Literal file names**: `run_git` sets `GIT_LITERAL_PATHSPECS=1`, so a name such as + `[ab].txt` stages only that file instead of also matching `a.txt` and `b.txt`. +- **No option injection**: `--remote` / `--branch` (and values from config or the + upstream) are rejected if empty, starting with `-`, or containing spaces or control + characters (`validate_ref_part`, checked in `cli.validate` and `git.resolve_target`), + and `--` precedes the remote in `push`, `ls-remote` and `remote get-url`. +- **Untrusted repository config**: `config.apply_config` ignores `SENSITIVE_KEYS` + (`force`, `remote`, `branch`, `report`, `log_file`, `notify_webhook`, `include_from`, + `exclude_from`) unless the file was passed with `--config`. The ignored names are + printed and logged. +- **Credentials**: `redact()` hides `user:password@` in URLs. It is applied by the log + formatter (so also to tracebacks), to git and push error messages, to the push output + shown on the console, and to the report's `error` field. +- **Private files**: log and report files are restricted to the owner (`0600`, best effort + on Windows). +- **Rule injection**: `lfs.attribute_line` refuses file names with control characters, so + a newline cannot add attribute lines. +- **Report temp file**: `RunReport.write` uses `tempfile.mkstemp` (random name, created + exclusively) in the target folder, then `os.replace`. +- **Rich markup**: file names, commit text, remote output and error messages are shown as + `Text` or passed through `escape()`, so brackets such as `[/x]` neither crash the display + nor disappear. + ## Logging `logs.setup_logging` (called from `cli.main`) attaches one `FileHandler` to the diff --git a/pyproject.toml b/pyproject.toml index dbd430c..28e646e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "commitrocket" -version = "0.1.0" +version = "0.1.1" description = "Batch Git changes by size, commit each batch, and push with live progress." readme = "README.md" license = { file = "LICENSE" } diff --git a/src/commitrocket/__init__.py b/src/commitrocket/__init__.py index 243c56c..39956ba 100644 --- a/src/commitrocket/__init__.py +++ b/src/commitrocket/__init__.py @@ -1,3 +1,3 @@ """CommitRocket: batch Git changes by size, commit each batch, and push.""" -__version__ = "0.1.0" +__version__ = "0.1.1" diff --git a/src/commitrocket/cli.py b/src/commitrocket/cli.py index ccec711..dc96bff 100644 --- a/src/commitrocket/cli.py +++ b/src/commitrocket/cli.py @@ -3,11 +3,13 @@ import sys from rich.console import Console +from rich.markup import escape from commitrocket import __version__ from commitrocket.batching import BATCH_TYPES, DETERMINISTIC from commitrocket.config import ConfigError, apply_config, find_config from commitrocket.doctor import run_doctor +from commitrocket.errors import CommitRocketError from commitrocket.lfs import LFS_THRESHOLD_MIB from commitrocket.logs import ( DEFAULT_LEVEL, @@ -20,6 +22,7 @@ from commitrocket.options import RunOptions from commitrocket.report import RunReport from commitrocket.runner import run +from commitrocket.security import validate_ref_part from commitrocket.template import DEFAULT_TEMPLATE, PLACEHOLDERS, validate_template DEFAULT_LIMIT_MIB = 100 @@ -260,6 +263,12 @@ def validate(parser, args): if args.lfs_commit_rules and not args.use_lfs: parser.error("--lfs-commit-rules requires --use-lfs") + for label, value in (("--remote", args.remote), ("--branch", args.branch)): + if value is not None: + try: + validate_ref_part(value, label) + except CommitRocketError as exc: + parser.error(str(exc)) if args.notify_webhook and not is_valid_webhook(args.notify_webhook): parser.error("--notify-webhook must be an http:// or https:// URL") @@ -284,10 +293,14 @@ def run_main(argv): known, _ = pre.parse_known_args(argv) config_path = None + ignored_settings = [] try: config_path, values = find_config(known.repo, known.config) if values: - apply_config(parser, values, config_path) + # Only a file named with --config is trusted with sensitive settings. + ignored_settings = apply_config( + parser, values, config_path, trusted=bool(known.config) + ) except ConfigError as exc: parser.error(str(exc)) @@ -330,7 +343,20 @@ def run_main(argv): "[yellow]Could not open a log file; continuing without one.[/yellow]" ) if config_path: - console.print(f"[dim]Using config: {config_path}[/dim]") + console.print(f"[dim]Using config: {escape(str(config_path))}[/dim]") + if ignored_settings: + names = ", ".join(ignored_settings) + console.print( + f"[yellow]Ignored from {escape(str(config_path))}: {escape(names)}. " + "Settings that skip the prompt, change where files are pushed or " + "written, or contact other servers are only used from a file you " + "pass with --config.[/yellow]" + ) + logging.getLogger(__name__).warning( + "Ignored sensitive settings from untrusted config %s: %s", + config_path, + names, + ) report = RunReport(options) try: diff --git a/src/commitrocket/config.py b/src/commitrocket/config.py index 19a2782..cd99b53 100644 --- a/src/commitrocket/config.py +++ b/src/commitrocket/config.py @@ -9,6 +9,20 @@ # Options that make no sense in a config file. NOT_CONFIGURABLE = {"config", "repo", "help", "version"} +# Options an untrusted (repository) config file must not set: they can skip the +# confirmation, push to another remote, write files, send data to a URL or read +# files. They work in a file passed explicitly with --config. +SENSITIVE_KEYS = { + "force", + "remote", + "branch", + "report", + "log_file", + "notify_webhook", + "include_from", + "exclude_from", +} + class ConfigError(CommitRocketError): """The config file could not be read or holds an invalid value.""" @@ -52,19 +66,31 @@ def _load(path): return data -def apply_config(parser, values, source): - """Use config values as the parser's defaults, so the command line wins.""" +def apply_config(parser, values, source, trusted=False): + """Use config values as the parser's defaults, so the command line wins. + + A config file found in the repository is untrusted: anyone who can change the + repository can change it. Unless trusted (the user named it with --config), + settings that can skip the confirmation, push elsewhere, write files or contact + other servers are ignored. Returns the names of the ignored settings. + """ actions = {action.dest: action for action in parser._actions} defaults = {} + ignored = [] for key, value in values.items(): dest = key.replace("-", "_") action = actions.get(dest) if action is None or dest in NOT_CONFIGURABLE: raise ConfigError(f"{source}: unknown option '{key}'") - defaults[dest] = _coerce(action, key, value, source) + coerced = _coerce(action, key, value, source) + if dest in SENSITIVE_KEYS and not trusted: + ignored.append(key) + else: + defaults[dest] = coerced parser.set_defaults(**defaults) + return ignored def _coerce(action, key, value, source): diff --git a/src/commitrocket/doctor.py b/src/commitrocket/doctor.py index 1e24479..9e8f6c3 100644 --- a/src/commitrocket/doctor.py +++ b/src/commitrocket/doctor.py @@ -4,6 +4,7 @@ from pathlib import Path from rich.table import Table +from rich.text import Text from commitrocket.batching import MIB from commitrocket.config import ConfigError, find_config @@ -21,6 +22,7 @@ from commitrocket.journal import Journal from commitrocket.lfs import HOST_LIMIT_MIB, is_lfs_available from commitrocket.logs import default_log_dir +from commitrocket.security import redact logger = logging.getLogger(__name__) @@ -31,7 +33,7 @@ def _first_line(text): lines = [line for line in text.strip().splitlines() if line.strip()] - return lines[0] if lines else "no details" + return redact(lines[0]) if lines else "no details" def _remote_check(repo, args, label): @@ -120,12 +122,12 @@ def add(status, name, detail): add(SKIP, "Push permission", "skipped (--offline)") else: status, detail = _remote_check( - root, ["ls-remote", target.remote, "HEAD"], "remote reachable" + root, ["ls-remote", "--", target.remote, "HEAD"], "remote reachable" ) add(status, "Remote access", detail) status, detail = _remote_check( root, - ["push", "--dry-run", target.remote, target.refspec], + ["push", "--dry-run", "--", target.remote, target.refspec], "push accepted (dry run)", ) add(WARN if status == FAIL else status, "Push permission", detail) @@ -184,7 +186,8 @@ def run_doctor( table.add_column("Check") table.add_column("Details", overflow="fold") for status, name, detail in results: - table.add_row(f"[{STYLES[status]}]{status.upper()}[/]", name, detail) + label = f"[{STYLES[status]}]{status.upper()}[/]" + table.add_row(label, Text(name), Text(detail)) console.print(table) failed = [name for status, name, _ in results if status == FAIL] diff --git a/src/commitrocket/git.py b/src/commitrocket/git.py index d47f465..85e98cd 100644 --- a/src/commitrocket/git.py +++ b/src/commitrocket/git.py @@ -1,13 +1,17 @@ import logging +import os import subprocess from dataclasses import dataclass from pathlib import Path from commitrocket.errors import CommitRocketError, GitError +from commitrocket.security import redact, validate_ref_part logger = logging.getLogger(__name__) MAX_LOGGED_OUTPUT = 2000 +MAX_LOGGED_COMMAND = 500 +MAX_COMMAND_CHARS = 20000 def _logged_output(result): @@ -23,18 +27,55 @@ def run_git(repo, *args, check=True): ["git", "-C", str(repo), *args], text=True, capture_output=True, + # File names are literal: "[ab].txt" must not also match a.txt and b.txt. + env={**os.environ, "GIT_LITERAL_PATHSPECS": "1"}, ) + command = _shortened(" ".join(args)) logger.debug( - "git %s -> exit %d%s", " ".join(args), result.returncode, _logged_output(result) + "git %s -> exit %d%s", command, result.returncode, _logged_output(result) ) if check and result.returncode != 0: error = result.stderr.strip() or result.stdout.strip() - raise GitError(f"git {' '.join(args)} failed:\n{error}") + raise GitError(redact(f"git {command} failed:\n{error}")) return result +def _shortened(command): + """Keep logs and errors readable when a command lists thousands of files.""" + if len(command) <= MAX_LOGGED_COMMAND: + return command + return command[:MAX_LOGGED_COMMAND] + "... [truncated]" + + +def chunk_paths(paths, limit=MAX_COMMAND_CHARS): + """Split paths into groups that fit on one command line. + + Windows allows about 32,767 characters for a whole command, so a batch with + thousands of files cannot be passed to git in one go. Each path is counted at + twice its length, to leave room for the quoting Windows adds. + """ + chunk = [] + size = 0 + for path in paths: + cost = 2 * len(path) + 3 + if chunk and size + cost > limit: + yield chunk + chunk = [] + size = 0 + chunk.append(path) + size += cost + if chunk: + yield chunk + + +def add_files(repo, paths): + """Stage the given files, however many there are.""" + for chunk in chunk_paths(paths): + run_git(repo, "add", "--", *chunk) + + def get_paths(repo, *args): return [path for path in run_git(repo, *args).stdout.split("\0") if path] @@ -93,7 +134,8 @@ def refspec(self): @property def label(self): - return f"{self.remote}/{self.branch}" + """Display name for the target; safe to show, log, report and send.""" + return redact(f"{self.remote}/{self.branch}") def resolve_target(repo, local_branch, remote=None, branch=None): @@ -114,6 +156,10 @@ def resolve_target(repo, local_branch, remote=None, branch=None): branch = branch or configured_merge.removeprefix("refs/heads/") or local_branch + # These end up on git command lines, so they must not look like options. + validate_ref_part(remote, "remote") + validate_ref_part(branch, "branch") + tracking_ref = f"refs/remotes/{remote}/{branch}" exists = run_git( repo, "rev-parse", "--verify", "--quiet", tracking_ref, check=False @@ -129,12 +175,12 @@ def resolve_target(repo, local_branch, remote=None, branch=None): def get_remote_url(repo, remote): - return run_git(repo, "remote", "get-url", remote).stdout.strip() + return run_git(repo, "remote", "get-url", "--", remote).stdout.strip() def get_remote_head(repo, remote, ref): """Commit the remote currently has for ref, or None if it doesn't exist.""" - fields = run_git(repo, "ls-remote", remote, ref).stdout.split() + fields = run_git(repo, "ls-remote", "--", remote, ref).stdout.split() return fields[0] if fields else None diff --git a/src/commitrocket/lfs.py b/src/commitrocket/lfs.py index d50f62d..36efe3c 100644 --- a/src/commitrocket/lfs.py +++ b/src/commitrocket/lfs.py @@ -7,6 +7,7 @@ from commitrocket.errors import CommitRocketError from commitrocket.git import get_file_size, run_git +from commitrocket.security import has_control_chars # Git hosts reject pushes containing files larger than this. HOST_LIMIT_MIB = 100 @@ -29,6 +30,12 @@ def find_lfs_candidates(repo, files, threshold_bytes, max_bytes): def attribute_line(path): """A .gitattributes-style rule that sends exactly this file through LFS.""" + if has_control_chars(path): + # A newline here would let a file name add rules of its own. + raise CommitRocketError( + f"Cannot create an LFS rule for {path!r}: the file name contains " + "control characters. Rename the file and try again." + ) glob_escaped = re.sub(r"([*?\[\]\\])", r"\\\1", path) quoted = glob_escaped.replace("\\", "\\\\").replace('"', '\\"') return f'"/{quoted}" filter=lfs diff=lfs merge=lfs -text' diff --git a/src/commitrocket/logs.py b/src/commitrocket/logs.py index 6e51691..25faca4 100644 --- a/src/commitrocket/logs.py +++ b/src/commitrocket/logs.py @@ -4,11 +4,20 @@ from datetime import datetime from pathlib import Path +from commitrocket.security import redact, restrict_permissions + LOG_LEVELS = ("DEBUG", "INFO", "WARNING", "ERROR") DEFAULT_LEVEL = "INFO" LOGGER_NAME = "commitrocket" +class RedactingFormatter(logging.Formatter): + """Hides credentials embedded in URLs, including in tracebacks.""" + + def format(self, record): + return redact(super().format(record)) + + def default_log_dir(): """The conventional per-user log folder for the current OS.""" home = Path.home() @@ -43,8 +52,9 @@ def setup_logging(level=DEFAULT_LEVEL, log_file=None): logger.addHandler(logging.NullHandler()) return None + restrict_permissions(path) handler.setFormatter( - logging.Formatter("%(asctime)s %(levelname)-7s %(name)s: %(message)s") + RedactingFormatter("%(asctime)s %(levelname)-7s %(name)s: %(message)s") ) logger.addHandler(handler) return path.resolve() diff --git a/src/commitrocket/notify.py b/src/commitrocket/notify.py index 254e4c8..988ddb2 100644 --- a/src/commitrocket/notify.py +++ b/src/commitrocket/notify.py @@ -17,6 +17,7 @@ INTERRUPTED, SUCCESS, ) +from commitrocket.security import redact logger = logging.getLogger(__name__) @@ -72,7 +73,7 @@ def build_message(report): if report.duration_seconds is not None: parts.append(f"in {format_duration(report.duration_seconds)}") - return title, "; ".join(parts) or "No changes were made." + return title, redact("; ".join(parts) or "No changes were made.") def _run_quietly(command, env): @@ -124,7 +125,7 @@ def notify_webhook(url, title, body, report): "status": report.status, "exit_code": report.exit_code, "repo": report.repo, - "target": report.target, + "target": redact(report.target) if report.target else None, "batches": len(report.batches), "files": report.total_files, "size_bytes": report.total_bytes, diff --git a/src/commitrocket/push.py b/src/commitrocket/push.py index 8c9d75b..01fceb4 100644 --- a/src/commitrocket/push.py +++ b/src/commitrocket/push.py @@ -4,7 +4,10 @@ import subprocess import time +from rich.markup import escape + from commitrocket.errors import PushError +from commitrocket.security import redact logger = logging.getLogger(__name__) @@ -131,7 +134,7 @@ def push_with_progress( process = subprocess.Popen( ["git", "-C", str(repo), "push", "--progress"] - + ([remote, refspec] if remote else []), + + (["--", remote, refspec] if remote else []), stdout=subprocess.PIPE, stderr=subprocess.STDOUT, bufsize=0, @@ -223,7 +226,7 @@ def push_with_progress( progress.update(push_task, description="Git push failed", visible=True) raise PushError( "git push failed with exit code " - f"{return_code}:\n" + "\n".join(messages[-15:]), + f"{return_code}:\n" + redact("\n".join(messages[-15:])), retryable=is_retryable("\n".join(messages)), ) @@ -238,6 +241,6 @@ def push_with_progress( # Display non-progress messages such as the remote branch update. for line in messages: if not PERCENT_RE.search(line): - console.print(f"[dim]{line}[/dim]") + console.print(f"[dim]{escape(redact(line))}[/dim]") progress.update(push_task, visible=False) diff --git a/src/commitrocket/report.py b/src/commitrocket/report.py index e426fa9..592b49d 100644 --- a/src/commitrocket/report.py +++ b/src/commitrocket/report.py @@ -1,11 +1,13 @@ import json import os +import tempfile import time from dataclasses import asdict from datetime import UTC, datetime from pathlib import Path from commitrocket import __version__ +from commitrocket.security import redact_value, restrict_permissions SCHEMA_VERSION = 1 @@ -33,7 +35,8 @@ def __init__(self, options=None): self.branch = None self.target = None self.log_file = None - self.settings = asdict(options) if options else {} + # --remote may be a URL with embedded credentials. + self.settings = redact_value(asdict(options)) if options else {} self.resumed = {"staged_files": 0, "unpushed_commits": 0} self.lfs = {"files": 0, "rules_commit": None} self.batches = [] @@ -109,10 +112,19 @@ def write(self, path): """Write the report as JSON, replacing any file there in one step.""" path = Path(path).expanduser() path.parent.mkdir(parents=True, exist_ok=True) - temporary = path.with_name(path.name + ".tmp") - temporary.write_text( - json.dumps(self.to_dict(), indent=2, ensure_ascii=False) + "\n", - encoding="utf-8", + text = json.dumps(self.to_dict(), indent=2, ensure_ascii=False) + "\n" + + # A randomly named file created exclusively (and private to the owner), so + # nobody can pre-create a link at a predictable name to redirect the write. + descriptor, name = tempfile.mkstemp( + dir=path.parent, prefix=path.name + ".", suffix=".tmp" ) - os.replace(temporary, path) + try: + with os.fdopen(descriptor, "w", encoding="utf-8") as handle: + handle.write(text) + restrict_permissions(name) + os.replace(name, path) + except BaseException: + Path(name).unlink(missing_ok=True) + raise return path.resolve() diff --git a/src/commitrocket/runner.py b/src/commitrocket/runner.py index 5f1e5e4..a1307c9 100644 --- a/src/commitrocket/runner.py +++ b/src/commitrocket/runner.py @@ -2,7 +2,9 @@ import time from pathlib import Path +from rich.markup import escape from rich.panel import Panel +from rich.text import Text from commitrocket.batching import ( MIB, @@ -15,6 +17,7 @@ from commitrocket.filters import FileFilter from commitrocket.git import ( LocalConfig, + add_files, find_large_unpushed_blobs, get_file_size, get_pending_files, @@ -44,6 +47,7 @@ SUCCESS, RunReport, ) +from commitrocket.security import redact from commitrocket.template import render_message from commitrocket.ui import ( make_progress, @@ -71,11 +75,10 @@ def own_output_files(repo, options): if not raw: continue path = Path(raw).expanduser().resolve() - for candidate in (path, path.with_name(path.name + ".tmp")): - try: - found.add(candidate.relative_to(repo).as_posix()) - except ValueError: - pass # outside the repository + try: + found.add(path.relative_to(repo).as_posix()) + except ValueError: + pass # outside the repository return found @@ -186,7 +189,7 @@ def process_batches( if journal: journal.write(selected, commit_message, commit_number) - run_git(repo, "add", "--", *selected) + add_files(repo, selected) if not get_staged_files(repo): raise CommitRocketError("No staged changes to commit.") @@ -356,7 +359,9 @@ def list_pending(): ) status = run_git(repo, "status", "--short").stdout.strip() - console.print(Panel(status or "(no pending changes)", title="Pending Changes")) + console.print( + Panel(Text(status or "(no pending changes)"), title="Pending Changes") + ) if resume_staged or unpushed: logger.info( @@ -662,9 +667,10 @@ def check_lfs(selected): logger.warning("Interrupted by user") return 130 except (RuntimeError, OSError) as exc: - console.print(f"\n[bold red]Error:[/bold red] {exc}") + message = redact(str(exc)) + console.print(f"\n[bold red]Error:[/bold red] {escape(message)}") report.status = ABORTED if not acting else FAILED - report.error = str(exc) + report.error = message if acting: console.print( "[dim]Run the same command again to resume from where it " diff --git a/src/commitrocket/security.py b/src/commitrocket/security.py new file mode 100644 index 0000000..853a534 --- /dev/null +++ b/src/commitrocket/security.py @@ -0,0 +1,50 @@ +"""Small helpers that keep untrusted text from doing harm.""" + +import os +import re + +from commitrocket.errors import CommitRocketError + +# user:password@ inside a URL, e.g. https://name:token@host/path +_CREDENTIALS = re.compile(r"(?i)\b([a-z][a-z0-9+.\-]*://)[^/\s@]+@") +_CONTROL = re.compile(r"[\x00-\x1f\x7f]") + + +def redact(text): + """Hide credentials embedded in URLs before text is logged or shown.""" + return _CREDENTIALS.sub(r"\1***@", text) + + +def redact_value(value): + """redact() applied to every string inside nested dicts and lists.""" + if isinstance(value, str): + return redact(value) + if isinstance(value, dict): + return {key: redact_value(item) for key, item in value.items()} + if isinstance(value, (list, tuple)): + return [redact_value(item) for item in value] + return value + + +def has_control_chars(text): + return bool(_CONTROL.search(text)) + + +def validate_ref_part(value, label): + """Reject remote and branch names that Git could read as options.""" + if not value or value.startswith("-") or has_control_chars(value) or any( + char.isspace() for char in value + ): + raise CommitRocketError( + f"Invalid {label} {value!r}: it must not be empty, start with '-', " + "or contain spaces or control characters." + ) + return value + + +def restrict_permissions(path): + """Make a file readable and writable by its owner only (best effort).""" + try: + os.chmod(path, 0o600) + except OSError: + pass diff --git a/src/commitrocket/ui.py b/src/commitrocket/ui.py index 2c2cb89..fdf541c 100644 --- a/src/commitrocket/ui.py +++ b/src/commitrocket/ui.py @@ -1,5 +1,6 @@ import time +from rich.markup import escape from rich.panel import Panel from rich.progress import ( BarColumn, @@ -48,9 +49,9 @@ def print_run_info(console, repo, branch, upstream, limit_mib, max_mib, number): info = Table.grid(padding=(0, 2)) info.add_column(style="bold cyan") info.add_column() - info.add_row("Repository", str(repo)) - info.add_row("Branch", branch) - info.add_row("Push target", upstream) + info.add_row("Repository", Text(str(repo))) + info.add_row("Branch", Text(branch)) + info.add_row("Push target", Text(upstream)) info.add_row("Normal batch limit", f"{limit_mib} MiB") info.add_row("Maximum file size", f"{max_mib} MiB") info.add_row("Starting commit #", str(number)) @@ -64,7 +65,7 @@ def print_lfs_required(console, repo, paths, threshold_mib): table.add_column("Size (MiB)", justify="right") for path in sorted(paths): - table.add_row(path, f"{get_file_size(repo, path) / MIB:.2f}") + table.add_row(Text(path), f"{get_file_size(repo, path) / MIB:.2f}") console.print(table) console.print( @@ -88,7 +89,8 @@ def print_resume(console, staged, entry, unpushed): if len(unpushed) > 10: lines.append(f" ... and {len(unpushed) - 10} more") - console.print(Panel("\n".join(lines), title="Resuming", border_style="yellow")) + panel = Panel(Text("\n".join(lines)), title="Resuming", border_style="yellow") + console.print(panel) def print_dry_run( @@ -107,7 +109,7 @@ def print_dry_run( lfs_count = len(set(paths) & set(lfs_files)) table.add_row( str(index + 1), - messages[index], + Text(messages[index]), str(len(paths)), f"{size / MIB:.2f}", str(lfs_count) if lfs_count else "-", @@ -128,7 +130,7 @@ def print_dry_run( details.add_column("File", overflow="fold") details.add_column("Size (MiB)", justify="right") for path, size in sorted(deferred): - details.add_row(path, f"{size / MIB:.2f}") + details.add_row(Text(path), f"{size / MIB:.2f}") console.print(details) if no_push: @@ -146,19 +148,19 @@ def print_stuck_commits(console, commits, blobs, upstream, threshold_mib): table.add_column("Size (MiB)", justify="right") for path, size in blobs: - table.add_row(path, f"{size / MIB:.2f}") + table.add_row(Text(path), f"{size / MIB:.2f}") console.print(table) console.print(f"\n[bold]{len(commits)} unpushed local commit(s):[/bold]") for commit in commits: - console.print(f" {commit}") + console.print(Text(f" {commit}")) console.print( f"\n[bold red]These commits already contain files over {threshold_mib} " "MiB, so pushing them would be rejected.[/bold red]\n" "[yellow]Nothing was changed. To drop those commits and keep your files " "as untracked changes, run:[/yellow]\n\n" - f" git reset --mixed {upstream}\n\n" + f" git reset --mixed {escape(upstream)}\n\n" "[yellow]then run CommitRocket again (the commits stay recoverable via " "'git reflog').[/yellow]" ) @@ -200,7 +202,7 @@ def print_final_summary( for path, size in sorted(deferred): details.add_row( - path, + Text(path), f"{size / MIB:.2f}", f"Exceeds --max-mib ({max_mib} MiB)", ) diff --git a/tests/test_cli.py b/tests/test_cli.py index 39545a1..dd95554 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -12,16 +12,10 @@ def test_own_output_files_are_found_only_inside_the_repo(tmp_path): log_file=str(repo / "logs" / "run.log"), report=str(tmp_path / "elsewhere" / "report.json"), ) - assert own_output_files(repo.resolve(), options) == { - "logs/run.log", - "logs/run.log.tmp", - } + assert own_output_files(repo.resolve(), options) == {"logs/run.log"} options = RunOptions(report=str(repo / "report.json")) - assert own_output_files(repo.resolve(), options) == { - "report.json", - "report.json.tmp", - } + assert own_output_files(repo.resolve(), options) == {"report.json"} assert own_output_files(repo.resolve(), RunOptions()) == set() @@ -33,6 +27,8 @@ def test_own_output_files_are_found_only_inside_the_repo(tmp_path): ["--message", "m", "--message-template", "{nope}"], ["--message", "m", "--lfs-threshold-mib", "150"], ["--message", "m", "--retries", "-1"], + ["--message", "m", "--remote", "--receive-pack=x"], + ["--message", "m", "--remote", "origin", "--branch", "-d"], [], ], ) diff --git a/tests/test_config.py b/tests/test_config.py index 762d8fc..90e434e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -73,6 +73,54 @@ def test_bad_config_is_rejected(tmp_path, toml): parse(tmp_path, toml) +SENSITIVE = ( + 'force = true\nremote = "evil"\nbranch = "evil"\nreport = "/tmp/x"\n' + 'log-file = "/tmp/y"\nnotify-webhook = "https://evil.example/h"\n' + 'include-from = ["/etc/passwd"]\nexclude-from = ["/etc/passwd"]\n' + "retries = 9\n" +) + + +def load(tmp_path, toml, trusted): + (tmp_path / "commitrocket.toml").write_text(toml) + parser = build_parser() + path, values = find_config(tmp_path) + ignored = apply_config(parser, values, path, trusted=trusted) + return parser.parse_args([]), ignored + + +def test_untrusted_config_cannot_set_sensitive_options(tmp_path): + args, ignored = load(tmp_path, SENSITIVE, trusted=False) + + assert sorted(ignored) == sorted( + [ + "force", + "remote", + "branch", + "report", + "log-file", + "notify-webhook", + "include-from", + "exclude-from", + ] + ) + assert args.force is False + assert args.remote is None and args.branch is None + assert args.report is None and args.log_file is None + assert args.notify_webhook is None + assert args.include_from is None and args.exclude_from is None + assert args.retries == 9 # ordinary settings still apply + + +def test_explicit_config_may_set_sensitive_options(tmp_path): + args, ignored = load(tmp_path, SENSITIVE, trusted=True) + + assert ignored == [] + assert args.force is True + assert args.remote == "evil" + assert args.notify_webhook == "https://evil.example/h" + + def test_explicit_config_must_exist(tmp_path): with pytest.raises(ConfigError): find_config(tmp_path, tmp_path / "missing.toml") diff --git a/tests/test_git.py b/tests/test_git.py index 1afe3c0..b09ff40 100644 --- a/tests/test_git.py +++ b/tests/test_git.py @@ -3,6 +3,8 @@ from commitrocket.errors import CommitRocketError, GitError from commitrocket.git import ( LocalConfig, + add_files, + chunk_paths, get_pending_files, get_staged_files, resolve_target, @@ -38,6 +40,47 @@ def test_local_config_sets_and_restores(git_repo): assert unset.returncode != 0 +def test_chunk_paths_keeps_order_and_fits_the_limit(): + paths = [f"folder/file-{index:05d}.txt" for index in range(500)] + + chunks = list(chunk_paths(paths, limit=2000)) + + assert [path for chunk in chunks for path in chunk] == paths + assert len(chunks) > 1 + assert all(sum(2 * len(p) + 3 for p in chunk) <= 2000 for chunk in chunks) + + +def test_chunk_paths_handles_edge_cases(): + assert list(chunk_paths([])) == [] + # One path longer than the limit still goes through, alone. + assert list(chunk_paths(["x" * 50, "y"], limit=10)) == [["x" * 50], ["y"]] + + +def test_add_files_stages_thousands_of_files(git_repo): + # ~4,000 long names are far beyond a single Windows command line (32,767 chars). + folder = git_repo / "a-folder-with-a-reasonably-long-name" + folder.mkdir() + names = [f"a-reasonably-long-file-name-number-{i:05d}.txt" for i in range(4000)] + for name in names: + (folder / name).write_text("x") + paths = [f"{folder.name}/{name}" for name in names] + + add_files(git_repo, paths) + + assert len(get_staged_files(git_repo)) == 4000 + + +def test_long_commands_are_shortened_in_errors(git_repo): + # About 24,000 characters: long enough to truncate, short enough to run. + paths = [f"missing-file-{i:05d}.txt" for i in range(1000)] + + with pytest.raises(GitError) as raised: + run_git(git_repo, "add", "--", *paths) + + assert len(str(raised.value)) < 1500 + assert "[truncated]" in str(raised.value) + + def test_resolve_target_needs_a_remote(git_repo): with pytest.raises(CommitRocketError, match="No upstream"): resolve_target(git_repo, "main") diff --git a/tests/test_security.py b/tests/test_security.py new file mode 100644 index 0000000..907f4dc --- /dev/null +++ b/tests/test_security.py @@ -0,0 +1,197 @@ +import io +import logging +import os +import stat +import sys + +import pytest +from rich.console import Console + +from commitrocket import logs, notify +from commitrocket.errors import CommitRocketError +from commitrocket.git import resolve_target, run_git +from commitrocket.lfs import attribute_line +from commitrocket.options import RunOptions +from commitrocket.report import RunReport +from commitrocket.security import ( + has_control_chars, + redact, + redact_value, + restrict_permissions, + validate_ref_part, +) +from commitrocket.ui import print_lfs_required + + +@pytest.mark.parametrize( + ("text", "expected"), + [ + ( + "To https://user:ghp_secret@github.com/o/r.git", + "To https://***@github.com/o/r.git", + ), + ("ssh://git:pw@host/repo", "ssh://***@host/repo"), + ("https://github.com/o/r.git", "https://github.com/o/r.git"), + ("https://example.com/a@b", "https://example.com/a@b"), + ("git@github.com:o/r.git", "git@github.com:o/r.git"), + ], +) +def test_redact(text, expected): + assert redact(text) == expected + + +def test_control_characters(): + assert has_control_chars("a\nb") and has_control_chars("a\x00b") + assert not has_control_chars("normal name (1).mp4") + + +@pytest.mark.parametrize("value", ["--receive-pack=x", "-x", "", "a b", "a\nb"]) +def test_unsafe_remote_and_branch_names_are_rejected(value): + with pytest.raises(CommitRocketError): + validate_ref_part(value, "remote") + + +def test_normal_names_are_accepted(): + assert validate_ref_part("origin", "remote") == "origin" + assert validate_ref_part("feat/x-1", "branch") == "feat/x-1" + + +def test_resolve_target_rejects_option_like_names(git_repo): + with pytest.raises(CommitRocketError): + resolve_target(git_repo, "main", remote="--upload-pack=x") + with pytest.raises(CommitRocketError): + resolve_target(git_repo, "main", remote="origin", branch="-d") + + +def test_git_treats_file_names_literally(git_repo): + for name in ("a.txt", "b.txt", "[ab].txt"): + (git_repo / name).write_text(name) + + run_git(git_repo, "add", "--", "[ab].txt") + + staged = run_git(git_repo, "diff", "--cached", "--name-only").stdout.split() + assert staged == ["[ab].txt"] + + +CREDENTIAL_URL = "https://me:supersecret@example.com/org/repo.git" + + +def test_target_label_hides_credentials_in_a_url_remote(git_repo): + target = resolve_target(git_repo, "main", remote=CREDENTIAL_URL, branch="main") + + assert "supersecret" not in target.label + assert target.label.startswith("https://***@example.com/") + # The real value is still what git is given. + assert target.remote == CREDENTIAL_URL + + +def test_report_never_contains_a_credentialed_remote(git_repo, tmp_path): + options = RunOptions(message="m", remote=CREDENTIAL_URL, branch="main") + report = RunReport(options) + target = resolve_target(git_repo, "main", CREDENTIAL_URL, "main") + report.target = target.label + report.error = redact(f"push to {CREDENTIAL_URL} failed") + report.finish(1) + + path = report.write(tmp_path / "report.json") + + assert "supersecret" not in path.read_text(encoding="utf-8") + + +def test_notifications_never_contain_a_credentialed_remote(git_repo): + options = RunOptions(message="m", remote=CREDENTIAL_URL, branch="main") + report = RunReport(options) + report.target = resolve_target(git_repo, "main", CREDENTIAL_URL, "main").label + report.add_batch(1, "m #1", ["a"], 10) + report.finish(0) + + title, body = notify.build_message(report) + assert "supersecret" not in title + body + + # Even if an unredacted target slipped in, the outgoing payload cleans it. + report.target = CREDENTIAL_URL + sent = {} + + def fake_urlopen(request, timeout): + sent["body"] = request.data.decode("utf-8") + raise OSError("stop here") + + original = notify.urllib.request.urlopen + notify.urllib.request.urlopen = fake_urlopen + try: + with pytest.raises(OSError): + notify.notify_webhook("https://hooks.example/x", title, body, report) + finally: + notify.urllib.request.urlopen = original + + assert "supersecret" not in sent["body"] + + +def test_redact_value_cleans_nested_data(): + data = {"a": [CREDENTIAL_URL, {"b": CREDENTIAL_URL}], "n": 3, "none": None} + + assert "supersecret" not in str(redact_value(data)) + assert redact_value(data)["n"] == 3 + + +def test_lfs_rules_refuse_file_names_with_control_characters(): + with pytest.raises(CommitRocketError): + attribute_line("x\n* filter=lfs") + + +def test_log_file_never_contains_url_credentials(tmp_path): + path = logs.setup_logging("DEBUG", tmp_path / "run.log") + try: + logging.getLogger("commitrocket.test").info( + "remote is https://me:supersecret@example.com/r.git" + ) + try: + raise RuntimeError("failed for https://me:supersecret@example.com/r.git") + except RuntimeError: + logging.getLogger("commitrocket.test").exception("push failed") + finally: + logs.close_logging() + + text = path.read_text(encoding="utf-8") + assert "supersecret" not in text + assert "https://***@example.com/r.git" in text + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX permission bits") +def test_private_files(tmp_path): + path = tmp_path / "secret.txt" + path.write_text("x") + os.chmod(path, 0o644) + + restrict_permissions(path) + + assert stat.S_IMODE(path.stat().st_mode) == 0o600 + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX permission bits") +def test_report_and_log_are_private(tmp_path): + report_path = RunReport().write(tmp_path / "report.json") + assert stat.S_IMODE(report_path.stat().st_mode) == 0o600 + + log_path = logs.setup_logging("INFO", tmp_path / "run.log") + logs.close_logging() + assert stat.S_IMODE(log_path.stat().st_mode) == 0o600 + + +def test_report_leaves_no_temporary_files(tmp_path): + RunReport().write(tmp_path / "report.json") + assert [p.name for p in tmp_path.iterdir()] == ["report.json"] + + +def test_square_brackets_in_file_names_are_shown_literally(tmp_path): + # Rich would otherwise read "[/x]" as markup and fail, or swallow "[HD]". + # The files need not exist: a missing file is simply shown with size 0. + names = ["[/x] clip.mp4", "Intro [HD].mp4"] + + output = io.StringIO() + console = Console(file=output, width=200, force_terminal=False) + print_lfs_required(console, tmp_path, names, 100) + + text = output.getvalue() + assert "[/x] clip.mp4" in text + assert "Intro [HD].mp4" in text diff --git a/uv.lock b/uv.lock index b56bd91..80f575f 100644 --- a/uv.lock +++ b/uv.lock @@ -22,7 +22,7 @@ wheels = [ [[package]] name = "commitrocket" -version = "0.1.0" +version = "0.1.1" source = { editable = "." } dependencies = [ { name = "pathspec" },