Skip to content

feat(rust): Disallows Debug format derivation and Debug formatting in release code. #423

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 5 commits into
base: master
Choose a base branch
from

Conversation

stevenj
Copy link
Collaborator

@stevenj stevenj commented Aug 5, 2025

Description

This change enforces that Debug formatting of structures can not be used in any release build.

This is achieved by using use_debug and also disallowd_macros

Why restrict this?

The purpose of the Debug trait is to facilitate debugging Rust code, and no guarantees are made about its output. It should not be used in user-facing output.

There is also a non zero risk of Debug formatted output leaking secrets or other information which can lead to vulnerabilities or security issues. They will also make any comprehensive security audit of the code difficult, if not impossible, as the output is uncontrolled.

This will also effect wrapping Errors by using {e:?} this is no longer permitted.
If an error is to be wrapped, it should impl Display and we should control what it produces when formatted for printing.

In preference complex structs should have a custom method attached (or impl Display) which allows the intended functional structure (without internal details) to be serialized to a json string. This allows this information to be properly embedded in logs, or later de-serialized and interpreted. This does not impose a requirement to allow all structs to have json de-serialization, and especially not serialization. It is suggested only where the structured contents of the structure are desirable for logging or reporting purposes.

Debug printing is useful during development, and there is no requirement to strip these logs or formatted prints from the code, instead they need to be wrapped to prevent them build built in release mode, but making them available to developers when debugging.

Example:

use clap::Parser;

/// Simple program to demonstrate debug exclusion from production builds
#[cfg_attr(debug_assertions, derive(Debug))]
#[derive(Parser)]
#[command(author, version, about, long_about = None)]
struct Args {
    /// Name of the person to greet
    #[arg(short, long)]
    name: String,

    /// Number of times to greet
    #[arg(short, long, default_value_t = 1)]
    count: u8,
}

/// The main entrypoint of this program.
fn main() {
    let args = Args::parse();

    #[cfg(debug_assertions)]
    println!("{args:?}");
}

Breaking Changes

Intentionally will break builds that use this in release builds.

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@stevenj stevenj self-assigned this Aug 5, 2025
@stevenj stevenj marked this pull request as draft August 5, 2025 06:23
@stevenj stevenj marked this pull request as ready for review August 5, 2025 06:23
@stevenj stevenj moved this from New to 👀 In review in Catalyst Aug 5, 2025
@stevenj stevenj added the ci/cd CI/CD Fixes or Improvements. label Aug 5, 2025
Mr-Leshiy
Mr-Leshiy previously approved these changes Aug 5, 2025
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy left a comment

Choose a reason for hiding this comment

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

LGTM

rafal-ch
rafal-ch previously approved these changes Aug 5, 2025
Copy link
Contributor

@rafal-ch rafal-ch left a comment

Choose a reason for hiding this comment

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

LGTM 👍

stanislav-tkach
stanislav-tkach previously approved these changes Aug 5, 2025
@stevenj stevenj dismissed stale reviews from stanislav-tkach, Mr-Leshiy, and rafal-ch via 706ca5e August 6, 2025 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/cd CI/CD Fixes or Improvements.
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

5 participants