Skip to content

fix: correct permissions on root.json#133

Merged
SequeI merged 1 commit intodevelopfrom
asiek/permFix
Nov 3, 2025
Merged

fix: correct permissions on root.json#133
SequeI merged 1 commit intodevelopfrom
asiek/permFix

Conversation

@SequeI
Copy link
Member

@SequeI SequeI commented Nov 3, 2025

Summary by Sourcery

Set correct permissions on root.json in the TUF repository initialization script and add a directory listing for verification

Bug Fixes:

  • Ensure root.json is granted 644 file permissions after repository initialization

Enhancements:

  • Add recursive directory listing of the output directory for debugging and verification

@sourcery-ai
Copy link

sourcery-ai bot commented Nov 3, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds a step in the repository initialization script to set root.json permissions to 644 and includes a debug directory listing to verify outputs.

Flow diagram for updated repository initialization script

flowchart TD
    A["Delete old files"] --> B["Set 644 permissions on root.json"]
    B --> C["List contents of OUTDIR (debug)"]
    C --> D["Copy TUF repository to final location"]
Loading

File-Level Changes

Change Details Files
Enforce read/write permissions for root.json
  • Add echo message before setting permissions
  • Execute chmod 644 on root.json
rhtas/tuf-repo-init.sh
Add debug listing for output directory
  • Insert test comment
  • Add ls -Rla to display directory contents
rhtas/tuf-repo-init.sh

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 3, 2025

PR Compliance Guide 🔍

(Compliance updated until commit fbbb9f1)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
🟢
No codebase code duplication found No new components were introduced in the PR code
Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing Error Handling: The new commands (find/chmod and ls) lack error checking or handling for failures (e.g.,
missing OUTDIR, chmod errors), leading to potential silent failures.

Referred Code
echo "Setting 644 permissions on public repository files ..."
find "${OUTDIR}" -type f -exec chmod 644 {} +

#Test
ls -Rla "${OUTDIR}"
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Action Logging: Newly added permission changes and directory listing are not logged with user identity or
structured context, making audit trails incomplete.

Referred Code
echo "Setting 644 permissions on public repository files ..."
find "${OUTDIR}" -type f -exec chmod 644 {} +

#Test
ls -Rla "${OUTDIR}"
Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Verbose Output: The recursive 'ls -Rla' may expose internal directory structure and file names
to user-facing output, which could leak sensitive operational details.

Referred Code
#Test
ls -Rla "${OUTDIR}"
Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Unstructured Logs: The added echo and 'ls -Rla' produce unstructured output and may reveal file
names or paths that could be sensitive, lacking safeguards or redaction.

Referred Code
echo "Setting 644 permissions on public repository files ..."
find "${OUTDIR}" -type f -exec chmod 644 {} +

#Test
ls -Rla "${OUTDIR}"
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Path Validation: The new commands operate on '${OUTDIR}' without validating its value or ensuring
it points to the intended directory, which could risk unintended chmod on files.

Referred Code
echo "Setting 644 permissions on public repository files ..."
find "${OUTDIR}" -type f -exec chmod 644 {} +
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 30dbf16
Security Compliance
Information disclosure in logs

Description: Recursive ls -Rla "${OUTDIR}" may disclose directory structure and file metadata in logs;
consider guarding behind a debug flag or removing in production.
tuf-repo-init.sh [316-321]

Referred Code
echo "Setting 644 permissions on public repository files ..."
chmod 644 "${OUTDIR}/root.json"

#Test
ls -Rla "${OUTDIR}"
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
🟢
No codebase code duplication found No new components were introduced in the PR code
Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing error handling: The chmod and ls operations are executed without checking for failures or handling cases
where files are missing, leading to potential silent errors.

Referred Code
echo "Setting 644 permissions on public repository files ..."
chmod 644 "${OUTDIR}/root.json"

#Test
ls -Rla "${OUTDIR}"
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Incomplete audit context: The newly added permission change and repo listing are logged without user identity or
structured context, which may not meet audit trail requirements.

Referred Code
echo "Setting 644 permissions on public repository files ..."
chmod 644 "${OUTDIR}/root.json"

#Test
ls -Rla "${OUTDIR}"
Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Verbose directory listing: The recursive 'ls -Rla' may expose internal paths and permissions to stdout,
which could be user-visible depending on execution context.

Referred Code
#Test
ls -Rla "${OUTDIR}"
Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Unstructured logging: Echo and 'ls -Rla' produce unstructured logs and may include file names and
paths that could inadvertently reveal sensitive repository details.

Referred Code
echo "Setting 644 permissions on public repository files ..."
chmod 644 "${OUTDIR}/root.json"

#Test
ls -Rla "${OUTDIR}"

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `rhtas/tuf-repo-init.sh:319-320` </location>
<code_context>
+echo "Setting 644 permissions on public repository files ..."
+chmod 644 "${OUTDIR}/root.json"
+
+#Test
+ls -Rla "${OUTDIR}"

</code_context>

<issue_to_address>
**suggestion:** Remove or clarify the '#Test' comment for production readiness.

If ls is only needed for debugging, remove it or control its execution with a verbosity flag to keep production output clean.

```suggestion
if [ "${VERBOSE}" = "true" ]; then
    echo "Verbose mode enabled: listing contents of ${OUTDIR} ..."
    ls -Rla "${OUTDIR}"
fi
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 3, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Apply permissions to all repository files

The log message implies setting permissions for multiple files, but the chmod
command only targets root.json. To ensure all repository files are publicly
readable, use find to apply chmod 644 to all files within the ${OUTDIR}.

rhtas/tuf-repo-init.sh [316-317]

 echo "Setting 644 permissions on public repository files ..."
-chmod 644 "${OUTDIR}/root.json"
+find "${OUTDIR}" -type f -exec chmod 644 {} +
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential bug where only root.json has its permissions set, which could lead to access issues for other critical TUF repository files.

Medium
  • Update

@SequeI
Copy link
Member Author

SequeI commented Nov 3, 2025

/retest

Signed-off-by: SequeI <asiek@redhat.com>
Copy link
Member

@fghanmi fghanmi left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@SequeI SequeI merged commit bfa9291 into develop Nov 3, 2025
14 checks passed
fghanmi pushed a commit that referenced this pull request Jan 26, 2026
Signed-off-by: SequeI <asiek@redhat.com>
fghanmi pushed a commit that referenced this pull request Jan 26, 2026
Signed-off-by: SequeI <asiek@redhat.com>
fghanmi added a commit that referenced this pull request Jan 26, 2026
* chore: swap sha2 usage for openssl

* fix: correct permissions on root.json (#133)

Signed-off-by: SequeI <asiek@redhat.com>

* ci: add fips check

* remove unused import

---------

Signed-off-by: SequeI <asiek@redhat.com>
Co-authored-by: JasonPowr <japower@redhat.com>
Co-authored-by: Aleks <121458075+SequeI@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants