Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
376 lines (274 loc) · 15.1 KB

CONTRIBUTING.md

File metadata and controls

376 lines (274 loc) · 15.1 KB
Raw

Contributing to USBGuardGUI

First off, thank you for considering contributing to USBGuardGUI!

Table of Contents

Code of Conduct

By participating in this project you agree to abide by the Code of Conduct. Report unacceptable behavior to github.tgwp7@aleeas.com.

How to Report Bugs

  • Check existing issues — someone may have reported it already.
  • Use the issue template and provide as much detail as possible.
  • Include diagnostics: version, operating system, exact command, and full output.

How to Propose Features

  • Open an issue using the Feature request template.
  • Explain why the feature is needed and how it fits the project's goals, not only what it does.
  • Wait for maintainer feedback before investing significant implementation time.

Development Setup

  1. Clone the repository:

    git clone https://github.com/onyks-os/USBGuardGUI.git
cd USBGuardGUI

  2. Bootstrap the environment:

    make setup
cargo +stable install cargo-deny --version 0.20.2 --locked

  3. Install the Git pre-commit hook (runs lint + unit tests before each commit):

    make install-hooks

  4. Run the test suite:

    make test

Coding Standards

All contributions must satisfy the following before being submitted. These checks are enforced automatically by the CI pipeline.

  • Linting & formatting: the code must pass make lint with no errors or warnings.
  • Shell scripts: everything under scripts/ must pass shellcheck.
  • Type annotations: new functions and methods must be fully annotated, consistent with the existing codebase.
  • No dead code: remove unused imports, variables, and commented-out blocks before submitting.
  • Commit messages: follow Conventional Commits (feat:, fix:, docs:, refactor:, test:, chore:, ci:).

Pull requests that fail make lint will not be merged.

Architectural Principles

These are not general good advice; each one rules something out, and a pull request that breaks one will be asked to change. The reasoning behind each is in docs/architecture.md.

  1. The dependency direction is one way, and it is enforced. model depends on nothing else in the program; rules depends on model; dbus depends on model and rules; ui depends on all three. Nothing ever points back up. This is what makes model and rules testable without a bus and without a display, which in turn is what makes the majority of the test suite runnable in CI.

  2. The program acquires no privilege. No setuid, no capabilities, no helper daemon, and no read or write of any path under /etc, /var, or /sys. Every privileged effect is produced by the USBGuard daemon on its own authority. A feature that needs privilege is a feature that does not happen — propose it upstream instead.

  3. The two event loops never share state. The Tokio runtime owns every D-Bus interaction; the GLib main loop owns every widget. They exchange immutable UiEvent values over an executor-agnostic channel. A clippy.toml disallowed-methods lint enforces the boundary, and CI runs cargo clippy --all-targets -- -D warnings, so crossing it fails the build rather than producing a subtle bug at runtime.

  4. No optimistic updates. The view changes on a daemon signal or a confirmed reply, never in anticipation of one. A view that disagrees with the daemon invites the user to make a security decision on false information, which is worse than a view that is briefly out of date.

  5. Fail closed, and name the failure. When the program cannot establish what is true — a rule whose text no longer matches, an operation interrupted by a daemon restart, a denial it cannot attribute to a checkpoint — it refuses and says which, rather than picking the likely answer. "Service unavailable" is not an acceptable message: there are at least seven distinct reasons this program can fail to reach the daemon, and the remedy differs for each.

  6. The parser does not panic. Every byte it sees originates outside the program's control. No unwrap, no expect, no unbounded allocation, no input length left unchecked — enforced by lints, covered by property tests, and fuzzed in CI.

  7. Attacker-influenced text never becomes syntax. Rules are built as typed structures and rendered by the canonical quoter, never by string concatenation. A device name that contains a quote is a device name.

  8. Device identifiers stay out of logs. Names, serial numbers, and hashes are never logged above debug. A default-level log attached to a bug report must not be an inventory of the reporter's hardware.

  9. Every new feature or bug fix ships with a test, and a change to behaviour under failure ships with an entry in the resilience matrix in docs/verification.md.

Testing

  • Unit tests must pass on every pull request. They are fully mocked and require no privileges.
  • Integration tests run in a container or VM and verify real system behavior.

When Tests Run

  • Pull requests: every pull request triggers the CI pipeline — linting, static analysis, and the unit test matrix.
  • Push to main: the same suite runs on any push to the default branch.
  • Locally: run make verify before opening a pull request.

Interpreting Results

  • Green: ready for review.
  • Red: blocks the merge. Review the logs, fix, and push an update.

Test Policy for Major Changes

A change is major if it adds a significant feature, alters a security boundary, or changes the crash-safety architecture. For major changes the contributor must:

  • Add new unit tests covering the functionality.
  • Update existing tests when the expected behavior changes.
  • Run the integration suite manually and report the result in the pull request.

Pull requests are blocked from merging if tests do not sufficiently cover the change.

Documentation Requirements

Every change must keep the documentation in sync with the code. The exact files to update for each type of change are defined in docs/documentation-policy.md. At minimum, every user-visible change requires a CHANGELOG.md entry.

Developer Certificate of Origin (DCO)

By contributing to USBGuardGUI you certify that you have the right to submit the contribution under the project's MIT license, and you agree to the Developer Certificate of Origin v1.1.

Every commit must include a Signed-off-by line with your real name and email:

Signed-off-by: Jane Doe <jane@example.com>

The easiest way to add it is the -s flag:

git commit -s -m "your commit message"

For multiple commits in a branch, sign them off at once:

git rebase --signoff HEAD~<number-of-commits>

Pull requests with unsigned commits will not be merged. The DCO check is enforced by CI. The one exception is Dependabot, which cannot sign off: its commits are exempt, and a maintainer reviews and merges each of its pull requests.

Full DCO text 
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

Pull Request Process

  1. Create a branch from main.
  2. Ensure make verify passes locally.
  3. Update the documentation required by docs/documentation-policy.md.
  4. Fill in the pull request template checklist honestly.
  5. Submit the pull request and wait for review. Expect a first response within 7 days.

Thank you for your help!

Security Best Practices for GitHub Actions

When contributing workflows or modifying CI pipelines, follow these guidelines to prevent injection attacks.

1. Never interpolate untrusted data directly into shell commands

Bad (vulnerable to script injection):

- run: echo "PR title: ${{ github.event.pull_request.title }}"

Good (use environment variables):

- env:
    PR_TITLE: ${{ github.event.pull_request.title }}
  run: echo "PR title: $PR_TITLE"

2. Never combine pull_request_target with a checkout of the pull request

pull_request_target runs in the context of the base repository: it sees the repository secrets and gets a token that can write to this repository, even when the pull request comes from a fork. That is deliberate — it is the only way a fork's PR can be commented on or labelled. It becomes a full repository compromise the moment the same workflow also checks out the fork's code:

# NEVER DO THIS — a fork's code runs with a token that can push to this repository.
on: pull_request_target
permissions:
  pull-requests: write
jobs:
  test:
    steps:
      - uses: actions/checkout@...
        with:
          ref: ${{ github.event.pull_request.head.sha }}   # untrusted code
      - run: make test                                      # ...executed with write access

Anything the pull request controls counts as "the fork's code": not just ref: on the checkout, but build scripts, Makefile targets, test fixtures, package.json lifecycle hooks, and pre-commit configuration. OpenSSF Scorecard reports this pattern as Dangerous-Workflow.

How this usually gets introduced. A reporting job — SAST, SCA, coverage — is asked to comment its result on the pull request. That needs pull-requests: write, and on the pull_request trigger a fork's token is read-only regardless of the permissions: block, so the comment never appears. The first search result says to switch the trigger to pull_request_target, and the checkout is already sitting in the file. Nobody adds an exploit; the workflow just drifts into one.

Use one of these three instead.

  1. Do without the comment. Every reporting action already writes to $GITHUB_STEP_SUMMARY, which is visible on the checks tab and needs no write permission at all. This is what .github/workflows/dependency-review.yml does, and it is the right default.

  2. Split privilege from execution with workflow_run. The untrusted job runs under pull_request with no permissions and uploads its result as an artifact. A second workflow, triggered by workflow_run, holds the write permission, downloads that artifact, and posts the comment — without ever checking out the pull request:

    # report.yml — privileged, and touches no untrusted code.
on:
  workflow_run:
    workflows: ["CI"]
    types: [completed]
permissions:
  pull-requests: write
jobs:
  comment:
    runs-on: ubuntu-latest
    steps:
      # No actions/checkout of the head ref. Only the artifact is read,
      # and it is treated as data: never `run:` anything it contains.
      - uses: actions/download-artifact@...
        with:
          run-id: ${{ github.event.workflow_run.id }}
          github-token: ${{ secrets.GITHUB_TOKEN }}
          name: report

  3. pull_request_target with zero checkout of untrusted code. Legitimate when the job only reads pull request metadata through the API — labelling, size checks, the dependency graph. The rule is absolute: no actions/checkout of the head ref, and no execution of any file the pull request can change. If the workflow needs the PR's file contents, it is the wrong tool.

Whichever you pick, keep the privileged workflow short enough to audit in one screen, and remember that artifacts from an untrusted run are attacker-controlled data — parse them, never execute them.

3. Pin actions to a full commit SHA

Actions must be pinned to an immutable commit SHA, not a mutable tag or branch. A tag can be moved to point at different code after review; a SHA cannot. This applies to first-party actions too, because actions/checkout@v4 is just as mutable as any other tag:

- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0

Keep the human-readable version in a trailing comment — Dependabot reads it, and updates both the SHA and the comment together on the weekly github-actions run configured in .github/dependabot.yml.

A major bump of a first-party action is usually a runtime change — Node 20 to Node 24, say — rather than an input change. It is still a breaking change for self-hosted runners, which must meet the minimum runner version the release names before the workflow will run at all. GitHub-hosted runners are already there.

Actions that select their behaviour from the ref name need that behaviour restated as an input once the ref is a SHA. dtolnay/rust-toolchain@stable becomes a pinned SHA plus an explicit toolchain: stable, otherwise the pin silently changes which toolchain is installed.

4. Limit GITHUB_TOKEN permissions

Declare the minimum permissions at workflow level:

permissions:
  contents: read

5. Sanitize inputs from issue and comment bodies

If user-provided text must be used, validate it against an allowlist or escape it before passing it to a script.

6. Run untrusted code in isolated containers

For actions that execute code from pull requests, run them inside a container with no access to secrets.

Reference