insufficient locking in configx/provider.go

Question

insufficient locking in configx/provider.go

Opened this issue 2 years ago · 0 comments

Preflight checklist

I could not find a solution in the existing issues, docs, nor discussions.
I agree to follow this project's Code of Conduct.
I have read and am following this repository's Contribution Guidelines.
This issue affects my Ory Network project.
I have joined the Ory Community Slack.
I am signed up to the Ory Security Patch Newsletter.

Describe the bug

The functions to get a value from the provider, for example, provider.StringF(key, fallback) already lock the provider for reading:

https://github.com/ory/x/blob/master/configx/provider.go#L357-L366

However, calling provider.String(key) directly calls the embedded *koanf.Koanf, so the provider is not locked for reading.

Reproducing the bug

Use provider.String() and provider.Set concurrently, run with race detection.

Example: https://github.com/ory/keto/actions/runs/3994964134/jobs/6853285780#step:4:1447

Relevant log output

==================
WARNING: DATA RACE
Write at 0x00c0006a3558 by goroutine 6410:
  github.com/ory/x/configx.(*Provider).replaceKoanf()
      /home/runner/go/pkg/mod/github.com/ory/x@v0.0.533/configx/provider.go:205 +0x4b7
  github.com/ory/x/configx.(*Provider).Set()
      /home/runner/go/pkg/mod/github.com/ory/x@v0.0.533/configx/provider.go:331 +0x49b
  github.com/ory/keto/internal/driver/config.(*Config).Set()
      /home/runner/work/keto/keto/internal/driver/config/provider.go:157 +0x67
  github.com/ory/keto/internal/e2e.(*namespaceTestManager).add()
      /home/runner/work/keto/keto/internal/e2e/helpers.go:39 +0x211
  github.com/ory/keto/internal/e2e.runTransactionCases.func1.1()
      /home/runner/work/keto/keto/internal/e2e/transaction_cases_test.go:23 +0x1a4
  testing.tRunner()
      /opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:[1446](https://github.com/ory/keto/actions/runs/3994964134/jobs/6853285780#step:4:1447) +0x216
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1493 +0x47

Previous read at 0x00c0006a3558 by goroutine 6425:
  github.com/ory/keto/internal/driver/config.(*Config).addressFor()
      /home/runner/work/keto/keto/internal/driver/config/provider.go:170 +0x7a
  github.com/ory/keto/internal/driver/config.(*Config).MetricsListenOn()
      /home/runner/work/keto/keto/internal/driver/config/provider.go:177 +0x99
  github.com/ory/keto/internal/e2e.Test.func1.2.2()
      /home/runner/work/keto/keto/internal/e2e/full_suit_test.go:123 +0x6c
  testing.tRunner()
      /opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1493 +0x47

Goroutine 6410 (running) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1493 +0x75d
  github.com/ory/keto/internal/e2e.runTransactionCases.func1()
      /home/runner/work/keto/keto/internal/e2e/transaction_cases_test.go:21 +0x10e
  testing.tRunner()
      /opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1493 +0x47

Goroutine 6425 (finished) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1493 +0x75d
  github.com/ory/keto/internal/e2e.Test.func1.2()
      /home/runner/work/keto/keto/internal/e2e/full_suit_test.go:121 +0x2c4
  testing.tRunner()
      /opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.19.5/x64/src/testing/testing.go:1493 +0x47
==================

Relevant configuration

No response

Version

master

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

None

Additional Context

No response