Skip to content

Commit 6470017

Browse files
authored
refactor(difc): reduce boilerplate duplication in agent.go and labels.go (#2591)
Three structural duplication patterns in `internal/difc/` required updating multiple identical code blocks for any locking strategy, log format, or label type changes. ## Changes - **`modifyTags` helper** — bulk label mutations (`AddSecrecyTags`, `AddIntegrityTags`, `DropIntegrityTags`) were each inlining the same lock/operate/log pattern instead of using the existing `modifyTag` precedent: ```go func (a *AgentLabels) modifyTags(labelType, action, pastTense string, tags []Tag, fn func()) { logAgent.Printf("Agent %s %s %d %s tags", a.AgentID, action, len(tags), labelType) a.mu.Lock() defer a.mu.Unlock() fn() if len(tags) > 0 { log.Printf("[DIFC] Agent %s %s %s tags: %v", a.AgentID, pastTense, labelType, tags) } } // callers become one-liners: func (a *AgentLabels) AddSecrecyTags(tags []Tag) { a.modifyTags("secrecy", "adding", "gained", tags, func() { a.Secrecy.Label.AddAll(tags) }) } ``` - **`getTagsLocked` helper** — `GetSecrecyTags` and `GetIntegrityTags` each duplicated the same `RLock` + `GetTags()` body; extracted to a single nil-guarded helper. - **`cloneLabelOrNew` helper** — `SecrecyLabel.Clone` and `IntegrityLabel.Clone` shared identical nil-guard logic; extracted to a standalone function so both `Clone` methods become single-line delegations. > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build220454796/b334/launcher.test /tmp/go-build220454796/b334/launcher.test -test.testlogfile=/tmp/go-build220454796/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s rtcf�� ache/go/1.25.8/x-s 64/src/crypto/hm-w x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build988514342/b330/launcher.test /tmp/go-build988514342/b330/launcher.test -test.testlogfile=/tmp/go-build988514342/b330/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 1afb76c6d5a1b24a/run/containerd/io.containerd.runtime.v2.task/moby/c066a1e0c75e18fce6c91b9a92a66sed y e-handler by/2dfd37f3cd6c3git lang.org/x/text/config` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build220454796/b319/config.test /tmp/go-build220454796/b319/config.test -test.testlogfile=/tmp/go-build220454796/b319/testlog.txt -test.paniconexit0 -test.timeout=10m0s conf�� 64/src/runtime/cgo go x_amd64/compile` (dns block) > - Triggering command: `/tmp/go-build988514342/b315/config.test /tmp/go-build988514342/b315/config.test -test.testlogfile=/tmp/go-build988514342/b315/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true f6977767ec5dcecf/run/containerd/io.containerd.runtime.v2.task/moby/c066a1e0c75e18fce6c91b9a92a66/bin/sh runtime-runc/mob--log-format csi/net-interfacjson by/37580e7ab5c50git` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build220454796/b334/launcher.test /tmp/go-build220454796/b334/launcher.test -test.testlogfile=/tmp/go-build220454796/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s rtcf�� ache/go/1.25.8/x-s 64/src/crypto/hm-w x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build988514342/b330/launcher.test /tmp/go-build988514342/b330/launcher.test -test.testlogfile=/tmp/go-build988514342/b330/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 1afb76c6d5a1b24a/run/containerd/io.containerd.runtime.v2.task/moby/c066a1e0c75e18fce6c91b9a92a66sed y e-handler by/2dfd37f3cd6c3git lang.org/x/text/config` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build220454796/b334/launcher.test /tmp/go-build220454796/b334/launcher.test -test.testlogfile=/tmp/go-build220454796/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s rtcf�� ache/go/1.25.8/x-s 64/src/crypto/hm-w x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build988514342/b330/launcher.test /tmp/go-build988514342/b330/launcher.test -test.testlogfile=/tmp/go-build988514342/b330/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 1afb76c6d5a1b24a/run/containerd/io.containerd.runtime.v2.task/moby/c066a1e0c75e18fce6c91b9a92a66sed y e-handler by/2dfd37f3cd6c3git lang.org/x/text/config` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build220454796/b343/mcp.test /tmp/go-build220454796/b343/mcp.test -test.testlogfile=/tmp/go-build220454796/b343/testlog.txt -test.paniconexit0 -test.timeout=10m0s o_.o�� 64/src/net late/parse/lex.go x_amd64/vet -p github.com/tetra-test.testlogfile=/tmp/go-build220454796/b334/testlog.txt -lang=go1.24 x_amd64/vet 5991�� _.a 599188/b151/ x_amd64/vet --gdwarf-5 ternal/engine/wa-m -o dmA41Y_/RID864WwwFNZuxdU3EAU` (dns block) > - Triggering command: `/tmp/go-build988514342/b339/mcp.test /tmp/go-build988514342/b339/mcp.test -test.testlogfile=/tmp/go-build988514342/b339/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true /tmp/go-build220454796/b370/_pkg_.a by/ee0f09b892b0c84149e47062e494bfc467b029bcbe96e63e2b5ad75092fcc8cd ker/cli-plugins/docker-buildx by/ee0f09b892b0c/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet 2b5ad75092fcc8cd-atomic 76224512e8a2c58c-bool ker/cli-plugins/-buildtags n-me�� tes.crt -goversion /home/REDACTED/wor-nilfunc -c=4 -nolocalimports dea2d1d50bb3e29a-stringintconv 4w8uw1gybk6` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details> <!-- START COPILOT CODING AGENT TIPS --> --- 📱 Kick off Copilot coding agent tasks wherever you are with [GitHub Mobile](https://gh.io/cca-mobile-docs), available on iOS and Android.
2 parents 204ff33 + c665c6f commit 6470017

2 files changed

Lines changed: 51 additions & 35 deletions

File tree

internal/difc/agent.go

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,25 @@ func (a *AgentLabels) modifyTag(labelType, action, pastTense string, tag Tag, fn
5757
log.Printf("[DIFC] Agent %s %s %s tag: %s", a.AgentID, pastTense, labelType, tag)
5858
}
5959

60+
// modifyTags is a helper for bulk label mutations, analogous to modifyTag.
61+
// It handles the mutex lock, executes the modification action, and conditionally logs the operation.
62+
//
63+
// Parameters:
64+
// - labelType: The type of label being modified ("secrecy" or "integrity")
65+
// - action: The verb describing the action ("adding", "dropping", etc.)
66+
// - pastTense: The past tense verb for the operational log ("gained", "dropped", etc.)
67+
// - tags: The tags being modified
68+
// - fn: The function that performs the actual label modification
69+
func (a *AgentLabels) modifyTags(labelType, action, pastTense string, tags []Tag, fn func()) {
70+
logAgent.Printf("Agent %s %s %d %s tags", a.AgentID, action, len(tags), labelType)
71+
a.mu.Lock()
72+
defer a.mu.Unlock()
73+
fn()
74+
if len(tags) > 0 {
75+
log.Printf("[DIFC] Agent %s %s %s tags: %v", a.AgentID, pastTense, labelType, tags)
76+
}
77+
}
78+
6079
// AddSecrecyTag adds a secrecy tag to the agent
6180
func (a *AgentLabels) AddSecrecyTag(tag Tag) {
6281
a.modifyTag("secrecy", "adding", "gained", tag, func() {
@@ -80,35 +99,23 @@ func (a *AgentLabels) DropIntegrityTag(tag Tag) {
8099

81100
// DropIntegrityTags removes multiple integrity tags from the agent
82101
func (a *AgentLabels) DropIntegrityTags(tags []Tag) {
83-
logAgent.Printf("Agent %s dropping %d integrity tags", a.AgentID, len(tags))
84-
a.mu.Lock()
85-
defer a.mu.Unlock()
86-
a.Integrity.Label.RemoveAll(tags)
87-
if len(tags) > 0 {
88-
log.Printf("[DIFC] Agent %s dropped integrity tags: %v", a.AgentID, tags)
89-
}
102+
a.modifyTags("integrity", "dropping", "dropped", tags, func() {
103+
a.Integrity.Label.RemoveAll(tags)
104+
})
90105
}
91106

92107
// AddSecrecyTags adds multiple secrecy tags to the agent
93108
func (a *AgentLabels) AddSecrecyTags(tags []Tag) {
94-
logAgent.Printf("Agent %s adding %d secrecy tags", a.AgentID, len(tags))
95-
a.mu.Lock()
96-
defer a.mu.Unlock()
97-
a.Secrecy.Label.AddAll(tags)
98-
if len(tags) > 0 {
99-
log.Printf("[DIFC] Agent %s gained secrecy tags: %v", a.AgentID, tags)
100-
}
109+
a.modifyTags("secrecy", "adding", "gained", tags, func() {
110+
a.Secrecy.Label.AddAll(tags)
111+
})
101112
}
102113

103114
// AddIntegrityTags adds multiple integrity tags to the agent
104115
func (a *AgentLabels) AddIntegrityTags(tags []Tag) {
105-
logAgent.Printf("Agent %s adding %d integrity tags", a.AgentID, len(tags))
106-
a.mu.Lock()
107-
defer a.mu.Unlock()
108-
a.Integrity.Label.AddAll(tags)
109-
if len(tags) > 0 {
110-
log.Printf("[DIFC] Agent %s gained integrity tags: %v", a.AgentID, tags)
111-
}
116+
a.modifyTags("integrity", "adding", "gained", tags, func() {
117+
a.Integrity.Label.AddAll(tags)
118+
})
112119
}
113120

114121
// ApplyPropagation applies label changes from a propagate-mode evaluation result
@@ -184,18 +191,24 @@ func (a *AgentLabels) Clone() *AgentLabels {
184191
}
185192
}
186193

187-
// GetSecrecyTags returns a copy of secrecy tags (thread-safe)
188-
func (a *AgentLabels) GetSecrecyTags() []Tag {
194+
// getTagsLocked returns a copy of tags from the given label under the agent's read lock.
195+
func (a *AgentLabels) getTagsLocked(label *Label) []Tag {
189196
a.mu.RLock()
190197
defer a.mu.RUnlock()
191-
return a.Secrecy.Label.GetTags()
198+
if label == nil {
199+
return nil
200+
}
201+
return label.GetTags()
202+
}
203+
204+
// GetSecrecyTags returns a copy of secrecy tags (thread-safe)
205+
func (a *AgentLabels) GetSecrecyTags() []Tag {
206+
return a.getTagsLocked(a.Secrecy.Label)
192207
}
193208

194209
// GetIntegrityTags returns a copy of integrity tags (thread-safe)
195210
func (a *AgentLabels) GetIntegrityTags() []Tag {
196-
a.mu.RLock()
197-
defer a.mu.RUnlock()
198-
return a.Integrity.Label.GetTags()
211+
return a.getTagsLocked(a.Integrity.Label)
199212
}
200213

201214
// AgentRegistry manages agent labels across all agents

internal/difc/labels.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,15 @@ func (l *Label) IsEmpty() bool {
150150
return len(l.tags) == 0
151151
}
152152

153+
// cloneLabelOrNew clones inner if it is non-nil, otherwise returns a new empty Label.
154+
// This helper centralizes the nil-guard logic shared by SecrecyLabel.Clone and IntegrityLabel.Clone.
155+
func cloneLabelOrNew(inner *Label) *Label {
156+
if inner == nil {
157+
return NewLabel()
158+
}
159+
return inner.Clone()
160+
}
161+
153162
// SecrecyLabel wraps Label with secrecy-specific flow semantics
154163
// Secrecy flow: data can only flow to contexts with equal or more secrecy tags
155164
// l ⊆ target (this has no tags that target doesn't have)
@@ -279,10 +288,7 @@ func (l *SecrecyLabel) CheckFlow(target *SecrecyLabel) (bool, []Tag) {
279288

280289
// Clone creates a copy of the secrecy label
281290
func (l *SecrecyLabel) Clone() *SecrecyLabel {
282-
if l.getLabel() == nil {
283-
return NewSecrecyLabel()
284-
}
285-
return &SecrecyLabel{Label: l.Label.Clone()}
291+
return &SecrecyLabel{Label: cloneLabelOrNew(l.getLabel())}
286292
}
287293

288294
// IntegrityLabel wraps Label with integrity-specific flow semantics
@@ -326,10 +332,7 @@ func (l *IntegrityLabel) CheckFlow(target *IntegrityLabel) (bool, []Tag) {
326332

327333
// Clone creates a copy of the integrity label
328334
func (l *IntegrityLabel) Clone() *IntegrityLabel {
329-
if l.getLabel() == nil {
330-
return NewIntegrityLabel()
331-
}
332-
return &IntegrityLabel{Label: l.Label.Clone()}
335+
return &IntegrityLabel{Label: cloneLabelOrNew(l.getLabel())}
333336
}
334337

335338
// ViolationType indicates what kind of DIFC violation occurred

0 commit comments

Comments
 (0)