Skip to content

Conversation

@SangameshV
Copy link
Contributor

fix: Improved console logging by redacting sensitive information from stdout and stderr (fixes #875 )

Please refer to the image below for a sample console output of the debricked-scan SSC FCLI action.
image

@rsenden
Copy link
Contributor

rsenden commented Dec 16, 2025

@copilot Please review the masking functionality in this PR, focusing on the following:

  • StdioMaskHelper seems to replicate logic that's already available in LogMaskHelper and FortifyCLIDynamicInitializer::registerDefaultLogMaskPatterns; any way to avoid this, potentially by modifying LogMaskHelper to be better suitable for stdio case? Stdio doesn't have log message type, so maybe we need to hold an extra map that contains all mask replacements? Also, are all replacements relevant for stdio, or should we just focus on replacements registered through CLI options and other user-provided data?
  • Related to the above, StdioMaskHelper doesn't provide conditional masking based on configured mask level
  • Any improvements possible for MaskingPrintStream? For example, why do we pass a new ByteArayOutputStream to the super constructor and then separately save a delegate; can't we just pass delegate to the super constructor and use it from there?
  • Any potential interference with generic stdout/stderr processing, AnsiConsole, and existing fcli stdout/stderr wrapping, for example in FcliCommandExecutor?
  • Any chance of stdout/stderr becoming corrupted, for example if fcli unexpectedly terminates through kill signal?
  • Any way to (further) optimize performance for StdioMaskHelper/LogMaskHelper?
  • Any other improvement suggestions?

Copy link
Contributor

Copilot AI commented Dec 16, 2025

@rsenden I've opened a new pull request, #890, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits December 16, 2025 10:18
…n issues

- Extend LogMaskHelper to support stdio masking with registerStdioPattern/registerStdioValue methods
- Add stdioPatternReplacer to LogMaskHelper for global stdio masking
- Refactor StdIoMaskHelper to delegate to LogMaskHelper instead of duplicating logic
- Implement conditional masking based on LogMaskLevel (respects --log-mask option)
- Fix MaskingPrintStream using inner MaskingOutputStream to avoid infinite recursion
- Register default stdio patterns in FortifyCLIDynamicInitializer focused on user-provided data

Co-authored-by: rsenden <8635138+rsenden@users.noreply.github.com>
Refactor stdio masking to reuse LogMaskHelper infrastructure and respect --log-mask level
@rsenden
Copy link
Contributor

rsenden commented Dec 16, 2025

@copilot Do we really need the stdio-specific patterns that are registered from FortifyCLIDynamicInitializer.java? These seem to be mostly related to potential HTTP responses, not user input. Also, I'm not sure whether any cli options (through MaskValue annotation or through mask setting in fcli action yaml files) are now being properly registered for redaction. For example, debricked-scan.yaml defines debrickedAccessToken with mask settings; these are being registered for log masking but possibly not for stdio masking? We'd want LogMaskingHelper to automatically also add these for stdio masking when being registered for log masking. If there's insufficient information as to whether a value/pattern registered for log masking should also be masked for stdio, assume it needs to be masked in both. If in general there's not sufficient info, just simplify the code to mask all registered values/patterns in both logs and stdio.

Copy link
Contributor

Copilot AI commented Dec 16, 2025

@rsenden I've opened a new pull request, #891, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 3 commits December 16, 2025 13:44
- Remove stdio-specific HTTP patterns from FortifyCLIDynamicInitializer
- Modify LogMaskHelper.registerValue() to automatically register for stdio masking
- Remove explicit StdIoMaskHelper.registerValue() calls from action processors
- Remove unused registerStdioPattern() method
- Simplify StdIoMaskHelper to focus only on install/uninstall operations

Co-authored-by: rsenden <8635138+rsenden@users.noreply.github.com>
Ensure URL-encoded sensitive values are masked in both logs and console output for consistency.

Co-authored-by: rsenden <8635138+rsenden@users.noreply.github.com>
Consolidate stdio and log masking to eliminate duplication
@rsenden rsenden merged commit b28d342 into dev/v3.x Dec 16, 2025
52 of 53 checks passed
@rsenden rsenden deleted the fcli-actions-fix branch December 16, 2025 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSC debricked-scan action outputs sensitive data

3 participants