Skip to content

Commit 496e43e

Browse files
xokdviumEricson2314
authored andcommitted
Restore isAllowed check in ChrootLinuxDerivationBuilder
This early return was lost in d4ef822. By doing some https://en.wikipedia.org/wiki/Non-virtual_interface_pattern, we can ensure that we don't make this mistake again --- implementations are no longer responsible for implementing the caching/memoization mechanism.
1 parent a786c9e commit 496e43e

File tree

3 files changed

+21
-7
lines changed

3 files changed

+21
-7
lines changed

src/libstore/include/nix/store/restricted-store.hh

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,21 @@ struct RestrictionContext
5252
* Add 'path' to the set of paths that may be referenced by the
5353
* outputs, and make it appear in the sandbox.
5454
*/
55-
virtual void addDependency(const StorePath & path) = 0;
55+
void addDependency(const StorePath & path)
56+
{
57+
if (isAllowed(path))
58+
return;
59+
addDependencyImpl(path);
60+
}
61+
62+
protected:
63+
64+
/**
65+
* This is the underlying implementation to be defined. The caller
66+
* will ensure that this is only called on newly added dependencies,
67+
* and that idempotent calls are a no-op.
68+
*/
69+
virtual void addDependencyImpl(const StorePath & path) = 0;
5670
};
5771

5872
/**

src/libstore/unix/build/derivation-builder.cc

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder
334334

335335
protected:
336336

337-
void addDependency(const StorePath & path) override;
337+
void addDependencyImpl(const StorePath & path) override;
338338

339339
/**
340340
* Make a file owned by the builder.
@@ -1203,11 +1203,8 @@ void DerivationBuilderImpl::stopDaemon()
12031203
daemonSocket.close();
12041204
}
12051205

1206-
void DerivationBuilderImpl::addDependency(const StorePath & path)
1206+
void DerivationBuilderImpl::addDependencyImpl(const StorePath & path)
12071207
{
1208-
if (isAllowed(path))
1209-
return;
1210-
12111208
addedPaths.insert(path);
12121209
}
12131210

src/libstore/unix/build/linux-derivation-builder.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -709,8 +709,11 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu
709709
DerivationBuilderImpl::killSandbox(getStats);
710710
}
711711

712-
void addDependency(const StorePath & path) override
712+
void addDependencyImpl(const StorePath & path) override
713713
{
714+
if (isAllowed(path))
715+
return;
716+
714717
auto [source, target] = ChrootDerivationBuilder::addDependencyPrep(path);
715718

716719
/* Bind-mount the path into the sandbox. This requires

0 commit comments

Comments
 (0)