refactor: use explicit octal notation (0oNNN) for file permissions#3161
refactor: use explicit octal notation (0oNNN) for file permissions#3161aicontentcreate2023-star wants to merge 1 commit intoconforma:mainfrom
Conversation
Fixes conforma#3160 Summary: Replace implicit octal file permission literals (0NNN) with explicit octal notation (0oNNN) across the codebase for improved readability and clarity. Changes: - Updated 46 files, 147 instances - 0644 → 0o644 (rw-r--r--) - 0755 → 0o755 (rwxr-xr-x) - 0600 → 0o600 (rw-------) The explicit 0o prefix makes it immediately clear that the number is octal, reducing ambiguity and potential misinterpretation.
📝 WalkthroughWalkthroughThis PR systematically updates octal permission literals throughout the codebase from legacy notation (e.g., 0755, 0644, 0600) to Go's explicit 0o-prefixed format (e.g., 0o755, 0o644, 0o600) for improved clarity and consistency. No functional or behavioral changes are introduced. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
Review Summary by QodoReplace implicit octal notation with explicit 0oNNN format across codebase
WalkthroughsDescription• Replace implicit octal literals (0NNN) with explicit notation (0oNNN) • Update 46 files with 147 instances of octal permission changes • Improve code clarity and align with modern Go formatting standards • No functional changes to file permissions or behavior Diagramflowchart LR
A["Implicit Octal<br/>0644, 0755, 0600"] -- "Refactor" --> B["Explicit Octal<br/>0o644, 0o755, 0o600"]
B --> C["Improved Readability<br/>and Clarity"]
File Changes1. acceptance/cli/cli_test.go
|
Code Review by Qodo
1. Regex test data corrupted
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| expected: expected, | ||
| right: `{ | ||
| "a": "0123", | ||
| "a": "0o123", |
There was a problem hiding this comment.
1. Regex test data corrupted 🐞 Bug ✓ Correctness
A JSON string value used to satisfy a digit-only regex was changed from "0123" to "0o123". This no longer matches the test’s expected regex (^0\d+$) and will cause the test case(s) to fail.
Agent Prompt
## Issue description
The octal-literal refactor modified JSON string test data from "0123" to "0o123", which no longer matches the test’s expected regex (^0\\d+$).
## Issue Context
This PR intends to update *file permission numeric literals* only. This change is inside a raw JSON string used as test input and should not be altered.
## Fix Focus Areas
- acceptance/cli/cli_test.go[30-74]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| { | ||
| Name: "Name (registry.io/repository/image:tag)", | ||
| Timestamp: "0001-01-01T00:00:00Z", | ||
| Timestamp: "0o001-01-01T00:00:00Z", |
There was a problem hiding this comment.
2. Junit timestamp string broken 🐞 Bug ✓ Correctness
A JUnit test now expects an invalid RFC3339 timestamp string ("0o001-01-01T00:00:00Z"). The
production code formats timestamps with RFC3339, so this expectation will not match and the test
will fail.
Agent Prompt
## Issue description
The unit test expectation for JUnit suite timestamp was modified to an invalid RFC3339 string ("0o001-...") which will not match the actual output from `r.created.Format(time.RFC3339)`.
## Issue Context
`toJUnit()` formats timestamps via RFC3339. The test uses the zero value for `created`, which formats with year "0001".
## Fix Focus Areas
- internal/applicationsnapshot/junit_test.go[158-165]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| assert.Regexp(t, `^Version v1 | ||
| Source ID abc | ||
| Change date 2009-11-10 23:00:00 \+0000 UTC \(\d{2} years ago\) | ||
| Change date 2009-11-10 23:00:00 \+0o000 UTC \(\d{2} years ago\) |
There was a problem hiding this comment.
3. Version regex offset broken 🐞 Bug ✓ Correctness
A unit test regex was changed to expect a timezone offset of '+0o000', which is not a valid time offset and will not match the string produced by printing a UTC time.
Agent Prompt
## Issue description
The VersionInfo stringer test regex was modified to expect a timezone offset formatted as '+0o000', which will not match the actual time string output.
## Issue Context
`VersionInfo.String()` prints `ChangedOn` directly (`%s` with `time.Time`). The test uses `time.UTC`, which yields a zero offset.
## Fix Focus Areas
- internal/version/version_test.go[41-46]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@acceptance/cli/cli_test.go`:
- Line 52: The test's expected regex '^0\\d+$' conflicts with the provided value
"0o123" (map entry "a": "0o123"), causing the modified:false case to fail;
either change the test data to use a matching string (e.g., "0123") or relax the
regex to accept the "0o" prefix (for example '^0(?:o)?\\d+$' or '^0[oO]?\\d+$')
so the value "0o123" satisfies the expectation; update the expectation string in
the test (the '^0\\d+$' literal) or the map entry "a" accordingly.
In `@internal/applicationsnapshot/junit_test.go`:
- Line 161: The test expectation contains an invalid timestamp string
"0o001-01-01T00:00:00Z" for the Timestamp field which will break assertions;
update the expected value to a valid RFC3339/ISO8601 string (e.g.,
"0001-01-01T00:00:00Z") in the test in
internal/applicationsnapshot/junit_test.go so the Timestamp field matches real
parsed/serialized timestamps used by the code under test.
In `@internal/tracker/tracker.go`:
- Line 39: Update the incorrect timestamp example in the comment inside
tracker.go: replace the malformed `0o001-01-01T00:00:00Z` string with the
correct ISO-like example `0001-01-01T00:00:00Z` so the comment accurately
documents the expected timestamp format (look for the comment containing "have a
value, e.g. 0o001-01-01T00:00:00Z.").
In `@internal/version/version_test.go`:
- Line 43: Update the regex in internal/version/version_test.go that currently
expects the invalid timezone string "+0o000 UTC" to instead expect the correct
Go timezone formatting "+0000 UTC"; locate the test assertion or pattern
variable in version_test.go that contains the text "+0o000 UTC" and replace that
substring with "+0000 UTC" so the regex matches the actual Go time output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d608e6d4-fec2-49fb-af9a-a24dcb78b3f4
📒 Files selected for processing (46)
acceptance/cli/cli_test.goacceptance/git/git.goacceptance/image/image.goacceptance/kubernetes/kind/image.goacceptance/pipeline/pipeline_definition.goacceptance/tekton/bundles.goacceptance/testenv/testenv.gobenchmark/internal/registry/registry.gobenchmark/offliner/offliner.gocmd/initialize/init_policies.gocmd/inspect/inspect_policy_test.gocmd/test/test.gocmd/track/track_bundle.gocmd/track/track_bundle_test.gocmd/validate/image_test.gocmd/validate/input_test.gointernal/applicationsnapshot/input_test.gointernal/applicationsnapshot/junit_test.gointernal/applicationsnapshot/vsa.gointernal/documentation/documentation.gointernal/evaluation_target/application_snapshot_image/application_snapshot_image.gointernal/evaluator/conftest_evaluator.gointernal/evaluator/conftest_evaluator_test_helpers.gointernal/evaluator/conftest_evaluator_unit_core_test.gointernal/evaluator/conftest_evaluator_unit_data_test.gointernal/evaluator/conftest_evaluator_unit_metadata_test.gointernal/evaluator/opa_evaluator_test.gointernal/input/validate_test.gointernal/kubernetes/client_test.gointernal/logging/logging.gointernal/opa/inspect_test.gointernal/policy/source/chooser_test.gointernal/policy/source/git_config_test.gointernal/policy/source/source.gointernal/policy/source/source_test.gointernal/tracker/tracker.gointernal/utils/helpers_test.gointernal/utils/oci/client.gointernal/utils/private_key_test.gointernal/validate/helpers_test.gointernal/validate/vsa/attest_test.gointernal/validate/vsa/file_retriever_test.gointernal/validate/vsa/storage_local.gointernal/validate/vsa/vsa.gointernal/validate/vsa/vsa_test.gointernal/version/version_test.go
| expected: expected, | ||
| right: `{ | ||
| "a": "0123", | ||
| "a": "0o123", |
There was a problem hiding this comment.
Test case now conflicts with its own regex expectation.
At Line 31, the expected pattern is ^0\\d+$, which does not match "0o123" (Lines 52 and 66). This can break the "passing" scenario’s modified: false expectation.
🔧 Proposed fix
- "a": "0o123",
+ "a": "0123",- "a": "0o123",
+ "a": "0123",Also applies to: 66-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@acceptance/cli/cli_test.go` at line 52, The test's expected regex '^0\\d+$'
conflicts with the provided value "0o123" (map entry "a": "0o123"), causing the
modified:false case to fail; either change the test data to use a matching
string (e.g., "0123") or relax the regex to accept the "0o" prefix (for example
'^0(?:o)?\\d+$' or '^0[oO]?\\d+$') so the value "0o123" satisfies the
expectation; update the expectation string in the test (the '^0\\d+$' literal)
or the map entry "a" accordingly.
| { | ||
| Name: "Name (registry.io/repository/image:tag)", | ||
| Timestamp: "0001-01-01T00:00:00Z", | ||
| Timestamp: "0o001-01-01T00:00:00Z", |
There was a problem hiding this comment.
Revert invalid timestamp string conversion in test expectation.
"0o001-01-01T00:00:00Z" is not a valid timestamp format and is unrelated to Go octal numeric literals. This can break the assertion.
Proposed fix
- Timestamp: "0o001-01-01T00:00:00Z",
+ Timestamp: "0001-01-01T00:00:00Z",As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Timestamp: "0o001-01-01T00:00:00Z", | |
| Timestamp: "0001-01-01T00:00:00Z", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/applicationsnapshot/junit_test.go` at line 161, The test expectation
contains an invalid timestamp string "0o001-01-01T00:00:00Z" for the Timestamp
field which will break assertions; update the expected value to a valid
RFC3339/ISO8601 string (e.g., "0001-01-01T00:00:00Z") in the test in
internal/applicationsnapshot/junit_test.go so the Timestamp field matches real
parsed/serialized timestamps used by the code under test.
| Ref string `json:"ref"` | ||
| // ExpiresOn should be omitted if there isn't a value. Not using a pointer means it will always | ||
| // have a value, e.g. 0001-01-01T00:00:00Z. | ||
| // have a value, e.g. 0o001-01-01T00:00:00Z. |
There was a problem hiding this comment.
Fix incorrect timestamp example in comment.
0o001-01-01T00:00:00Z is not a valid timestamp string; keep it as 0001-01-01T00:00:00Z to avoid misleading docs.
As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/tracker/tracker.go` at line 39, Update the incorrect timestamp
example in the comment inside tracker.go: replace the malformed
`0o001-01-01T00:00:00Z` string with the correct ISO-like example
`0001-01-01T00:00:00Z` so the comment accurately documents the expected
timestamp format (look for the comment containing "have a value, e.g.
0o001-01-01T00:00:00Z.").
| assert.Regexp(t, `^Version v1 | ||
| Source ID abc | ||
| Change date 2009-11-10 23:00:00 \+0000 UTC \(\d{2} years ago\) | ||
| Change date 2009-11-10 23:00:00 \+0o000 UTC \(\d{2} years ago\) |
There was a problem hiding this comment.
Regex expectation was changed to an invalid timezone format.
+0o000 UTC is not how Go formats timezone offsets; this should remain +0000 UTC.
Proposed fix
-Change date 2009-11-10 23:00:00 \+0o000 UTC \(\d{2} years ago\)
+Change date 2009-11-10 23:00:00 \+0000 UTC \(\d{2} years ago\)As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Change date 2009-11-10 23:00:00 \+0o000 UTC \(\d{2} years ago\) | |
| Change date 2009-11-10 23:00:00 \+0000 UTC \(\d{2} years ago\) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/version/version_test.go` at line 43, Update the regex in
internal/version/version_test.go that currently expects the invalid timezone
string "+0o000 UTC" to instead expect the correct Go timezone formatting "+0000
UTC"; locate the test assertion or pattern variable in version_test.go that
contains the text "+0o000 UTC" and replace that substring with "+0000 UTC" so
the regex matches the actual Go time output.
| expected: expected, | ||
| right: `{ | ||
| "a": "0123", | ||
| "a": "0o123", |
There was a problem hiding this comment.
What the bot said. This indicates the PR was created with a "dumb" search and replace. So there's some work needed to remove these incorrect string replacements.
|
@aicontentcreate2023-star there are some incorrect changes here, i.e. strings are changed that are not octal numbers, as pointed out by the review comments. Do you want try addressing that? |
2 participants
Fixes #3160
Summary
Replace implicit octal file permission literals (
0NNN) with explicit octal notation (0oNNN) across the codebase for improved readability and clarity.Changes
0644→0o644(rw-r--r--)0755→0o755(rwxr-xr-x)0600→0o600(rw-------)Motivation
The explicit
0oprefix makes it immediately clear that the number is octal, reducing ambiguity and potential misinterpretation. This aligns with modern Go formatting tools and improves code clarity.Testing