Skip to content

Commit 79eec0b

Browse files
committed
fix: prevent test DB leakage from TestShell_BeforeNode
NewApp().Before() unconditionally overwrites shell.Config with a fresh config using the default pgx driver, even when tests pre-set a config with the txdb driver for isolation. This causes BeforeNode to open real database connections, leaking keystore entries (encrypted_key_rings) to the shared test database. Concurrent tests like Test_CSAKeyStore_E2E then find unexpected keys and fail. Guard config creation in app.Before() with a nil check so pre-set test configs are preserved. Migrate TestShell_BeforeNode and TestShell_RunNode_WithBeforeNode to heavyweight.FullTestDBV2 for proper DB isolation between subtests while preventing leakage to other tests.
1 parent 2e5181a commit 79eec0b

2 files changed

Lines changed: 29 additions & 26 deletions

File tree

core/cmd/app.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,20 @@ func NewApp(s *Shell) *cli.App {
8080
// logger instead.
8181
lggr, closeFn := logger.NewLogger()
8282

83-
cfg, err := opts.New()
84-
if err != nil {
85-
return err
86-
}
87-
8883
s.Logger = lggr
8984
s.Registerer = prometheus.DefaultRegisterer // use the global DefaultRegisterer, should be safe since we only ever run one instance of the app per shell
9085
s.CloseLogger = closeFn
91-
s.Config = cfg
86+
87+
// Only create a new config from opts if one hasn't been pre-set.
88+
// Tests pre-set s.Config with a txdb-wrapped driver for DB isolation;
89+
// unconditionally overwriting it here causes DB writes to leak between tests.
90+
if s.Config == nil {
91+
cfg, err := opts.New()
92+
if err != nil {
93+
return err
94+
}
95+
s.Config = cfg
96+
}
9297

9398
if c.Bool("json") {
9499
s.Renderer = RendererJSON{Writer: os.Stdout}
@@ -122,11 +127,11 @@ func NewApp(s *Shell) *cli.App {
122127

123128
// Allow for initServerConfig to be called if the flag is provided.
124129
if c.Bool("applyInitServerConfig") {
125-
cfg, err = initServerConfig(&opts, s.configFiles, s.secretsFiles)
130+
serverCfg, err := initServerConfig(&opts, s.configFiles, s.secretsFiles)
126131
if err != nil {
127132
return err
128133
}
129-
s.Config = cfg
134+
s.Config = serverCfg
130135
}
131136

132137
return nil

core/cmd/shell_local_test.go

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,15 @@ func TestShell_RemoveBlocks(t *testing.T) {
507507

508508
func TestShell_BeforeNode(t *testing.T) {
509509
testutils.SkipShortDB(t)
510+
511+
// Use a dedicated test database so subtests share state without leaking
512+
// data to the shared test database. The "incorrect password" subtest relies
513+
// on a key ring created by the "correct password" subtest to verify that
514+
// decryption fails with the wrong password.
515+
cfg, _ := heavyweight.FullTestDBV2(t, func(c *chainlink.Config, s *chainlink.Secrets) {
516+
c.Insecure.OCRDevelopmentMode = nil
517+
})
518+
510519
tests := []struct {
511520
name string
512521
pwdfile string
@@ -519,14 +528,6 @@ func TestShell_BeforeNode(t *testing.T) {
519528

520529
for _, test := range tests {
521530
t.Run(test.name, func(t *testing.T) {
522-
cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) {
523-
s.Password.Keystore = models.NewSecret("dummy")
524-
c.EVM[0].Nodes[0].Name = ptr("fake")
525-
c.EVM[0].Nodes[0].HTTPURL = commonconfig.MustParseURL("http://fake.com")
526-
c.EVM[0].Nodes[0].WSURL = commonconfig.MustParseURL("WSS://fake.com/ws")
527-
c.Insecure.OCRDevelopmentMode = nil
528-
})
529-
530531
shell := cmd.Shell{
531532
Config: cfg,
532533
KeyStoreAuthenticator: cmd.TerminalKeyStoreAuthenticator{
@@ -577,6 +578,13 @@ func TestShell_BeforeNode(t *testing.T) {
577578
}
578579

579580
func TestShell_RunNode_WithBeforeNode(t *testing.T) {
581+
// Use a dedicated test database so subtests share state without leaking
582+
// data to the shared test database. The "incorrect password" subtest relies
583+
// on keystore entries from prior setup to verify decryption failure.
584+
cfg, db := heavyweight.FullTestDBV2(t, func(c *chainlink.Config, s *chainlink.Secrets) {
585+
c.Insecure.OCRDevelopmentMode = nil
586+
})
587+
580588
tests := []struct {
581589
name string
582590
pwdfile string
@@ -588,16 +596,6 @@ func TestShell_RunNode_WithBeforeNode(t *testing.T) {
588596

589597
for _, test := range tests {
590598
t.Run(test.name, func(t *testing.T) {
591-
cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) {
592-
s.Password.Keystore = models.NewSecret("dummy")
593-
c.EVM[0].Nodes[0].Name = ptr("fake")
594-
c.EVM[0].Nodes[0].HTTPURL = commonconfig.MustParseURL("http://fake.com")
595-
c.EVM[0].Nodes[0].WSURL = commonconfig.MustParseURL("WSS://fake.com/ws")
596-
// seems to be needed for config validate
597-
c.Insecure.OCRDevelopmentMode = nil
598-
})
599-
600-
db := pgtest.NewSqlxDB(t)
601599
keyStore := cltest.NewKeyStore(t, db)
602600
authProviderORM := localauth.NewORM(db, time.Minute, logger.TestLogger(t), audit.NoopLogger)
603601

0 commit comments

Comments
 (0)