open-policy-agent / opa

Open Policy Agent (OPA) is an open source, general-purpose policy engine.
https://www.openpolicyagent.org
Apache License 2.0
9.63k stars 1.33k forks source link

Absolute paths do not work with rego.Load on Windows #4521

Open lcarva opened 2 years ago

lcarva commented 2 years ago

Short description

When using rego.Load with an absolute path on Windows, the following error is returned when Rego.PrepareForEval is called:

1 error occurred during loading: CreateFile \Users\RUNNER~1\AppData\Local\Temp\TestRegoPolicyLoadAbsolutePath3698015745\001\policy.rego: The system cannot find the path specified.

The issue does not occur if a relative path is used. The issue does not work on Linux or OSX.

Steps To Reproduce

policy := `
    package signature

    allow {
        input.predicateType == "https://slsa.dev/provenance/v0.2"
    }
`
// This alternative line works:
// policyFile := "policy.rego"
policyFile := filepath.Join(t.TempDir(), "policy.rego")
if err := os.WriteFile(policyFile, []byte(policy), 0644); err != nil {
    t.Fatal(err)
}
r := rego.New(rego.Query("data.signature.allow"), rego.Load([]string{policyFile}, nil))

ctx := context.Background()

// This fails on Windows Server 2022.
_, err := r.PrepareForEval(ctx)
if err != nil {
    t.Fatal(err)
}

Expected behavior

PrepareForEval should not return an error.

Additional context

I created a small git repo to reproduce this issue. The test TestRegoPolicyLoadAbsolutePath fails, while the test TestRegoPolicyLoadRelativePath passes. The GitHub actions were useful in running the tests across different Operating Systems. I'm not sure how long the results will last, but here's a link to them.

stale[bot] commented 2 years ago

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

anderseknert commented 2 years ago

Hey @lcarva! Sorry for not acknowledging this before, but that definitely seems like a bug. If anyone with a Windows machine would consider taking a look at this, that would be much appreciated!

lcarva commented 2 years ago

@anderseknert, no worries! FWIW, it's possible to reproduce the issue with just GitHub actions as mentioned in the description. It's definitely not as easy to debug as a local system, but a potential way forward.

stale[bot] commented 2 years ago

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

lcarva commented 2 years ago

Still failing on opa v0.40.0 with the same error.

anderseknert commented 2 years ago

Yes, no changes here, @lcarva. The stalebot warning does not mean it's being closed/removed, just that there's not been any activity for some time.

stale[bot] commented 2 years ago

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

simar7 commented 1 year ago

Still seems to be the case on v0.54.0.

stale[bot] commented 1 year ago

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.

mariuspodean commented 2 weeks ago

I think the issue here spans over multiple areas were files need to be loaded; same error when trying to use certificates (--tls-cert-file, --tls-private-key-file, --tls-ca-cert-file) from an absolute path (v0.68)

mariuspodean commented 2 weeks ago

Possibly similar > https://github.com/open-policy-agent/opa/issues/6910

alexrohozneanu commented 2 weeks ago

In the scenario where we are running the opa executable on one drive and loading the files from another drive then files cannot be found. However, if the opa executable and the files to be loaded are in the same drive, everything is working.

opa run \
        -w --server \
        -b F:/opa/bundle.tar.gz \
        --log-level debug \
        --tls-cert-file C:/OPACerts/newcerts/SERVER.pem \
        --tls-private-key-file C:/OPACerts/keys/SERVER.key  \
        --tls-ca-cert-file C:/OPACerts/newcerts/CA.pem \
        --addr https://0.0.0.0:8181/ \
        --log-format=json-pretty \
        --set=decision_logs.console=true \
        --authentication=tls \
        --authorization=basic

It seems that in the following function we are loosing information about the prefix:

https://github.com/open-policy-agent/opa/blob/69cd3886ea2fd6ec167b5c6d935724303a794c48/internal/pathwatcher/utils.go#L100

On windows, if we comment out that line, files are loaded correctly.

@ashutosh-narkar Can you give us more details about why is the prefix ignored, since all the paths in windows have the following format {partition_letter}:/{actual_path}? e.g. C:/Folder/file

ashutosh-narkar commented 2 weeks ago

IIRC it may be assumed the watchers are set for path in the current directory. If you think there is scope for improvement here, feel free to submit a PR.

mariuspodean commented 2 weeks ago

IIRC it may be assumed the watchers are set for path in the current directory. If you think there is scope for improvement here, feel free to submit a PR.

I think there is, indeed, a need for a fix here; whether you opt for watches, or not, this is still called and basically cannot start the server on a Windows machine