Bug#845715: Required targets must not write outside of the source package tree

[Niels Thykier]

Sean Whitton:
> Hello,
> 
> On Sun 11 Nov 2018 at 09:10PM +0100, Bill Allombert wrote:
> 
>> Package support for TMPDIR can be introduced as a general requirement,
>> outside of the build process.
> 
> Okay.
> 
>> Maybe the proposal could be rewritten in a way that does not need to
>> cover the detail of temporaries files.
>>
>> How about:
>>
>> +Required targets must not attempt to write outside of the unpacked
>> +source package tree.  There are two exceptions.  Firstly, the binary
>> +targets may write the binary packages to the parent directory of the
>> +unpacked source package tree.  Secondly, required targets may write to
>> +/tmp, /var/tmp and to the directory specified by the ``TMPDIR`` environment
>> + variable, but must not depend on the content of either.
>> +
>> +This restriction is intended to prevent source package builds creating
>> +and depending on state outside of themselves, thus affecting multiple
>> +independent rebuilds.  In particular, the required targets must not
>> +attempt to write into ``HOME``.
> 
> Thank you for this text.  I'd be happy to second it, since it solves the
> problem I was trying to solve with my patch, but ideally I'd like to
> hear from those others who seconded the older patch to see if they are
> happy to drop the TMPDIR parts.
> 

Seconded, thanks.

~Niels




signature.asc
Description: OpenPGP digital signature

Bug#845715: Required targets must not write outside of the source package tree

[Sean Whitton]

Hello,

On Sun 11 Nov 2018 at 09:10PM +0100, Bill Allombert wrote:

> Package support for TMPDIR can be introduced as a general requirement,
> outside of the build process.

Okay.

> Maybe the proposal could be rewritten in a way that does not need to
> cover the detail of temporaries files.
>
> How about:
>
> +Required targets must not attempt to write outside of the unpacked
> +source package tree.  There are two exceptions.  Firstly, the binary
> +targets may write the binary packages to the parent directory of the
> +unpacked source package tree.  Secondly, required targets may write to
> +/tmp, /var/tmp and to the directory specified by the ``TMPDIR`` environment
> + variable, but must not depend on the content of either.
> +
> +This restriction is intended to prevent source package builds creating
> +and depending on state outside of themselves, thus affecting multiple
> +independent rebuilds.  In particular, the required targets must not
> +attempt to write into ``HOME``.

Thank you for this text.  I'd be happy to second it, since it solves the
problem I was trying to solve with my patch, but ideally I'd like to
hear from those others who seconded the older patch to see if they are
happy to drop the TMPDIR parts.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#845715: Required targets must not write outside of the source package tree

[Ian Jackson]

Stuart Prescott writes ("Bug#845715: Required targets must not write outside of 
the source package tree"):
> Bill Allombert wrote:
> > +This restriction is intended to prevent source package builds creating
> > +and depending on state outside of themselves, thus affecting multiple
> > +independent rebuilds.  In particular, the required targets must not
> > +attempt to write into ``HOME``.
> 
> At the risk of letting perfect be the enemy of good, is it obvious following 
> this final remark about HOME that:

Thanks for your attention to detail :-), but:

Yes, I think it is.  "In particular" introduces a statement which is
clarifies the meaning of the general rule, and assists the reader, by
giving an example.  I don't think "in particular" can be correctly
used to extend (or except from) a general rule in the way required by
the misreadings you are concerned about.

   "All felines have four legs.  In particular, cats do."
 * "All felines have four legs.  In particular, dogs do." <- wrong

> It's reasonably common to redefine HOME within d/rules to make the
> build robust against a user's config files and/or to prevent
> unwanted config files being created.

Having said all that, I don't know if it would be worth explicitly
mentioning this very general and useful technique for the benefit of
readers who haven't osmosed or reinvented it.

Thanks,
Ian.

-- 
Ian JacksonThese opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

Bug#845715: Required targets must not write outside of the source package tree

[Stuart Prescott]

Bill Allombert wrote:
> +Required targets must not attempt to write outside of the unpacked
> +source package tree.  There are two exceptions.  Firstly, the binary
> +targets may write the binary packages to the parent directory of the
> +unpacked source package tree.  Secondly, required targets may write to
> +/tmp, /var/tmp and to the directory specified by the ``TMPDIR``
> environment 
> + variable, but must not depend on the content of either.
> +
> +This restriction is intended to prevent source package builds creating
> +and depending on state outside of themselves, thus affecting multiple
> +independent rebuilds.  In particular, the required targets must not
> +attempt to write into ``HOME``.

At the risk of letting perfect be the enemy of good, is it obvious following 
this final remark about HOME that:

* if user sets TMPDIR=$HOME/tmp then it is indeed OK to write into HOME? 

* if the package redefines HOME within d/rules then it is ok to write to the 
(redefined) HOME?

It's reasonably common to redefine HOME within d/rules to make the build 
robust against a user's config files and/or to prevent unwanted config files 
being created.

https://codesearch.debian.net/search?q=path%3Adebian%2Frules+%5B+%5DHOME%3D

(I would cheerfully second the above text if my [deliberate] misreading is 
an outlier and any tightening of the text is so hard to understand that it's 
not actually an improvement.)

cheers
Stuart

-- 
Stuart Prescotthttp://www.nanonanonano.net/   stu...@nanonanonano.net
Debian Developer   http://www.debian.org/ stu...@debian.org
GPG fingerprint90E2 D2C1 AD14 6A1B 7EBB 891D BBC1 7EBB 1396 F2F7

Bug#845715: Required targets must not write outside of the source package tree

[Holger Levsen]

On Mon, Nov 12, 2018 at 09:32:51PM +0100, Bill Allombert wrote:
> > > I can be convinced otherwise with data, though.
> > :)
> If you still run 
> https://tests.reproducible-builds.org

we do, however, this setup is for testing for reproducible builds and
not trying 'random stuff' which might cause wide FTBFS problems, which
would be reported (quite prominently) to tracker.d.o and UDD.

> you could "chmod 1770 /tmp" and set TMPDIR to something valid and see
> how many packages FTBFS.

the above said, we are discussing "Vary setting TMPDIR during reproducibility
testing" in #913557 but that's also not trivial to do, see that bug for
details.


-- 
cheers,
Holger

---
   holger@(debian|reproducible-builds|layer-acht).org
   PGP fingerprint: B8BF 5413 7B09 D35C F026 FE9D 091A B856 069A AA1C


signature.asc
Description: PGP signature

Bug#845715: Required targets must not write outside of the source package tree

[Bill Allombert]

On Mon, Nov 12, 2018 at 12:09:20PM +, Holger Levsen wrote:
> On Sun, Nov 11, 2018 at 09:10:02PM +0100, Bill Allombert wrote:
> > More accurately: I am not sure the Debian archive is ready for these
> > bugs to be RC, especially since they are usually upstream bugs.
>  
> agreed & thanks for catching this.
> 
> > I can be convinced otherwise with data, though.
> 
> :)

If you still run 
https://tests.reproducible-builds.org
you could "chmod 1770 /tmp" and set TMPDIR to something valid and see
how many packages FTBFS.

Cheers,
-- 
Bill. 

Imagine a large red swirl here. 

Bug#845715: Required targets must not write outside of the source package tree

[Holger Levsen]

On Sun, Nov 11, 2018 at 09:10:02PM +0100, Bill Allombert wrote:
> More accurately: I am not sure the Debian archive is ready for these
> bugs to be RC, especially since they are usually upstream bugs.
 
agreed & thanks for catching this.

> I can be convinced otherwise with data, though.

:)

> How about:
> 
> +Required targets must not attempt to write outside of the unpacked
> +source package tree.  There are two exceptions.  Firstly, the binary
> +targets may write the binary packages to the parent directory of the
> +unpacked source package tree.  Secondly, required targets may write to
> +/tmp, /var/tmp and to the directory specified by the ``TMPDIR`` environment
> + variable, but must not depend on the content of either.
> +
> +This restriction is intended to prevent source package builds creating
> +and depending on state outside of themselves, thus affecting multiple
> +independent rebuilds.  In particular, the required targets must not
> +attempt to write into ``HOME``.

better indeed, thanks and secoded.


-- 
cheers,
Holger

---
   holger@(debian|reproducible-builds|layer-acht).org
   PGP fingerprint: B8BF 5413 7B09 D35C F026 FE9D 091A B856 069A AA1C


signature.asc
Description: PGP signature

Bug#845715: Required targets must not write outside of the source package tree

[Bill Allombert]

On Sun, Nov 11, 2018 at 11:25:58AM -0700, Sean Whitton wrote:
> Hello,
> 
> === Addressing the participants in this thread more generally,
> especially those who seconded my most recent patch: ===
> 
> Bill has raised several scenarios in which this new requirement could be
> interpreted as making a package RC-buggy, where that might be considered
> unreasonable.  Building in wording to avoid each of the cases would make
> the whole patch a lot more complex, but it could be done.
> 
> An alternative would be for us to weaken the main requirement of the
> patch from 'must' to 'should'.  That way, this patch would not be in the
> business of making any packages RC-buggy.  I don't think Bill thinks his
> cases are not bugs, just that they are not of RC severity.

More accurately: I am not sure the Debian archive is ready for these
bugs to be RC, especially since they are usually upstream bugs.

I can be convinced otherwise with data, though.

Other corner cases are /var/tmp and builds leading to files in /run or
/var to be created or modified. 

Package support for TMPDIR can be introduced as a general requirement,
outside of the build process.

> Concerns about (2): it seems to me that this would not reflect the
> project's consensus that source package builds really should not be
> writing to places outside of TMPDIR and their own trees, aside from the
> final generated binary packages.

Note that you used "should" :)

Maybe the proposal could be rewritten in a way that does not need to
cover the detail of temporaries files.

How about:

+Required targets must not attempt to write outside of the unpacked
+source package tree.  There are two exceptions.  Firstly, the binary
+targets may write the binary packages to the parent directory of the
+unpacked source package tree.  Secondly, required targets may write to
+/tmp, /var/tmp and to the directory specified by the ``TMPDIR`` environment
+ variable, but must not depend on the content of either.
+
+This restriction is intended to prevent source package builds creating
+and depending on state outside of themselves, thus affecting multiple
+independent rebuilds.  In particular, the required targets must not
+attempt to write into ``HOME``.

As far as RC bug are concerned, Policy need to reflect current practice
before the Project's consensus. If a change causes a lot of packages to
be RC buggy, then there is a need for a transition period during which
the bug is not RC and the maintainers are notified of the bugs and given
the opportunity to fix them. The alternative is that the policy
requirement is ignored.

Cheers,
-- 
Bill. 

Imagine a large red swirl here. 

Bug#845715: Required targets must not write outside of the source package tree

[Sean Whitton]

Hello,

=== Addressing the participants in this thread more generally,
especially those who seconded my most recent patch: ===

Bill has raised several scenarios in which this new requirement could be
interpreted as making a package RC-buggy, where that might be considered
unreasonable.  Building in wording to avoid each of the cases would make
the whole patch a lot more complex, but it could be done.

An alternative would be for us to weaken the main requirement of the
patch from 'must' to 'should'.  That way, this patch would not be in the
business of making any packages RC-buggy.  I don't think Bill thinks his
cases are not bugs, just that they are not of RC severity.

A third alternative is that we just keep the patch as it is now, and
rely on maintainers to exercise their judgement about the corner cases
that Bill describes.

So in summary, we can

1) make the patch a lot more complex, in order to reduce to bugs of
   normal severity situations where a package build: (i) harmlessly
   ignores TMPDIR, or (ii) leaves files in TMPDIR which do not affect
   further builds; or

2) weaken the main requirement from 'must' to 'should'; or

3) not explicitly account for Bill's cases.

Concerns about (1): Policy becomes a lot less useful if we complicate
the wording to build in all sorts of exceptions, where good judgement on
the part of package maintainers could handle those cases just fine.

I am apprehensive about the idea of distinguishing between 'should' and
'must' within the explicatation of the details of two exceptions to a
'must' requirement.  The gain seems so minimal for the loss of
readability.

Concerns about (2): it seems to me that this would not reflect the
project's consensus that source package builds really should not be
writing to places outside of TMPDIR and their own trees, aside from the
final generated binary packages.

So I am inclined towards (3) myself, but would very much like to hear
other's opinions.

=== Addressing Bill's most recent message in particular: ===

On Sun 11 Nov 2018 at 04:04PM +0100, Bill Allombert wrote:

>> > What about the severity of using /tmp even if TMPDIR is set ?
>> > I do not think it is RC outside of the build process so it would
>> > be inconvenient.
>>
>> The current wording makes that RC-buggy, indeed.
>>
>> What exactly do you mean by "not RC outside of the build process"?
>
> Debian policy does not mandate that packages support TMPDIR,
> so it is valid to package a program that does not support TMPDIR and
> always use /tmp instead.
>
> However if some other package use this program in its own build process,
> then this other package is RC buggy under this new rule. This can be
> inconvenient.

Okay, thanks, I see what you mean now.

> Do we have some data about how many packages fails to honor TMPDIR ?

Not to my own knowledge.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#845715: Required targets must not write outside of the source package tree

[Bill Allombert]

On Sun, Nov 11, 2018 at 07:42:15AM -0700, Sean Whitton wrote:
> Hello Bill,
> 
> On Sun 11 Nov 2018 at 11:41AM +0100, Bill Allombert wrote:
> 
> > I am not sure I see the difference. Do you imply something like
> >
> >  but files created in that directory SHOULD be deleted before the
> >  targets completes and MUST not be reused by subsequent executions of
> >  the target.
> 
> That's basically how I was reading it, yes.

Then this should be written to be less ambiguous.

> > What about the severity of using /tmp even if TMPDIR is set ?
> > I do not think it is RC outside of the build process so it would
> > be inconvenient.
> 
> The current wording makes that RC-buggy, indeed.
> 
> What exactly do you mean by "not RC outside of the build process"?

Debian policy does not mandate that packages support TMPDIR,
so it is valid to package a program that does not support TMPDIR and
always use /tmp instead.

However if some other package use this program in its own build process,
then this other package is RC buggy under this new rule. This can be
inconvenient.

Do we have some data about how many packages fails to honor TMPDIR ?

Cheers,
-- 
Bill. 

Imagine a large red swirl here. 

Bug#845715: Required targets must not write outside of the source package tree

[Sean Whitton]

Hello Bill,

On Sun 11 Nov 2018 at 11:41AM +0100, Bill Allombert wrote:

> I am not sure I see the difference. Do you imply something like
>
>  but files created in that directory SHOULD be deleted before the
>  targets completes and MUST not be reused by subsequent executions of
>  the target.

That's basically how I was reading it, yes.

> What about the severity of using /tmp even if TMPDIR is set ?
> I do not think it is RC outside of the build process so it would
> be inconvenient.

The current wording makes that RC-buggy, indeed.

What exactly do you mean by "not RC outside of the build process"?

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#845715: Required targets must not write outside of the source package tree

[Holger Levsen]

On Sat, Nov 10, 2018 at 08:38:07PM -0700, Sean Whitton wrote:
> diff --git a/policy/ch-source.rst b/policy/ch-source.rst
> index dc80243..3c6c9d5 100644
> --- a/policy/ch-source.rst
> +++ b/policy/ch-source.rst
> @@ -291,6 +291,20 @@ For packages in the main archive, no required targets 
> may attempt
>  network access, except, via the loopback interface, to services on the
>  build host that have been started by the build.
> 
> +Required targets must not attempt to write outside of the unpacked
> +source package tree.  There are two exceptions.  Firstly, the binary
> +targets may write the binary packages to the parent directory of the
> +unpacked source package tree.  Secondly, required targets may write to
> +the directory specified by the ``TMPDIR`` environment variable (or
> +``/tmp`` if that is not set), provided that files created in that
> +directory are deleted before the target completes and are not reused
> +by subsequent executions of the target.
> +
> +This restriction is intended to prevent source package builds creating
> +and depending on state outside of themselves, thus affecting multiple
> +independent rebuilds.  In particular, the required targets must not
> +attempt to write into ``HOME``.
> +

seconded, thanks.


-- 
cheers,
Holger

---
   holger@(debian|reproducible-builds|layer-acht).org
   PGP fingerprint: B8BF 5413 7B09 D35C F026 FE9D 091A B856 069A AA1C


signature.asc
Description: PGP signature

Bug#845715: Required targets must not write outside of the source package tree

[Bill Allombert]

On Sat, Nov 10, 2018 at 08:42:27PM -0700, Sean Whitton wrote:
> Hello,
> 
> On Sat 03 Nov 2018 at 11:42PM +0100, Bill Allombert wrote:
> 
> >> How about:
> >>
> >> As an exception, required targets may write to the directory specified
> >> by the ``TMPDIR`` environment variable (or ``/tmp`` if that is not
> >> set), provided that files created in that directory are deleted before
> >> the target completes and are not reused by subsequent executions of
> >> the target.
> >>
> >> This explicitly states that this is an environment variable and makes it
> >> clear that it refers to a directory.
> >
> > I may be wrong but I expect that a lot of packages targets leaves stray
> > files and directory in /tmp (that are created by mktemp but not removed)
> > but not reused, and also that some of them fail to honor TMPDIR.
> >
> > Do we have some data about this ?
> 
> There isn't any data about this that I know of.
> 
> The patch says
> 
> provided that files created in that directory are deleted before the
> target completes and are not reused by subsequent executions of the
> target.
> 
> rather than
> 
> but files created in that directory must be deleted before the
> targets completes and must not be reused by subsequent executions of
> the target.

I am not sure I see the difference. Do you imply something like

 but files created in that directory SHOULD be deleted before the
 targets completes and MUST not be reused by subsequent executions of
 the target.

> so I don't think applying this patch would make a package RC-buggy
> simply because it left some things behind in /tmp.

This is my concern, yes.

> On the other hand, if a package built differently the second time
> because of some things left behind in /tmp, that would surely already be
> considered to be a bug, possibly RC, depending on what the differences
> were.

Oh, of course I agree with that.

What about the severity of using /tmp even if TMPDIR is set ?
I do not think it is RC outside of the build process so it would
be inconvenient.

(note that if everything honor TMPDIR, the issue of stray tmpfiles
is easily fixed by 
export TMPDIR=`mktemp -d`
...
rm -r $TMPDIR
unset TMPDIR
)

Cheers,
-- 
Bill. 

Imagine a large red swirl here. 

Bug#845715: Required targets must not write outside of the source package tree

[Niels Thykier]

Sean Whitton:
> Hello,
> 
> On Fri 09 Nov 2018 at 09:46PM GMT, Niels Thykier wrote:
> 
>> I suspect we are missing an exception allowing the binary targets to
>> write the produced binaries in the parent directory of the unpacked
>> source tree.
>>   Otherwise pretty much all packages violate the policy when they
>> generate the actual .debs/.udebs. :)
> 
> Heh.  You're right.
> 
> Here is a new version of the patch, fixing this problem.  I am not sure
> that it is meaningful to require that this change be seconded, but out
> of (possibly too much) respect for process, seeking seconds (and CCing
> those who have already seconded in the hope they'll renew their
> seconds):
> 
> diff --git a/policy/ch-source.rst b/policy/ch-source.rst
> index dc80243..3c6c9d5 100644
> --- a/policy/ch-source.rst
> +++ b/policy/ch-source.rst
> @@ -291,6 +291,20 @@ For packages in the main archive, no required targets 
> may attempt
>  network access, except, via the loopback interface, to services on the
>  build host that have been started by the build.
> 
> +Required targets must not attempt to write outside of the unpacked
> +source package tree.  There are two exceptions.  Firstly, the binary
> +targets may write the binary packages to the parent directory of the
> +unpacked source package tree.  Secondly, required targets may write to
> +the directory specified by the ``TMPDIR`` environment variable (or
> +``/tmp`` if that is not set), provided that files created in that
> +directory are deleted before the target completes and are not reused
> +by subsequent executions of the target.
> +
> +This restriction is intended to prevent source package builds creating
> +and depending on state outside of themselves, thus affecting multiple
> +independent rebuilds.  In particular, the required targets must not
> +attempt to write into ``HOME``.
> +
>  The targets are as follows:
> 
>  ``build`` (required)
> 

Seconded, thanks.

~Niels




signature.asc
Description: OpenPGP digital signature

Bug#845715: Required targets must not write outside of the source package tree

[Sean Whitton]

Hello,

On Sat 03 Nov 2018 at 11:42PM +0100, Bill Allombert wrote:

>> How about:
>>
>> As an exception, required targets may write to the directory specified
>> by the ``TMPDIR`` environment variable (or ``/tmp`` if that is not
>> set), provided that files created in that directory are deleted before
>> the target completes and are not reused by subsequent executions of
>> the target.
>>
>> This explicitly states that this is an environment variable and makes it
>> clear that it refers to a directory.
>
> I may be wrong but I expect that a lot of packages targets leaves stray
> files and directory in /tmp (that are created by mktemp but not removed)
> but not reused, and also that some of them fail to honor TMPDIR.
>
> Do we have some data about this ?

There isn't any data about this that I know of.

The patch says

provided that files created in that directory are deleted before the
target completes and are not reused by subsequent executions of the
target.

rather than

but files created in that directory must be deleted before the
targets completes and must not be reused by subsequent executions of
the target.

so I don't think applying this patch would make a package RC-buggy
simply because it left some things behind in /tmp.

On the other hand, if a package built differently the second time
because of some things left behind in /tmp, that would surely already be
considered to be a bug, possibly RC, depending on what the differences
were.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#845715: Required targets must not write outside of the source package tree

[Sean Whitton]

Hello,

On Fri 09 Nov 2018 at 09:46PM GMT, Niels Thykier wrote:

> I suspect we are missing an exception allowing the binary targets to
> write the produced binaries in the parent directory of the unpacked
> source tree.
>   Otherwise pretty much all packages violate the policy when they
> generate the actual .debs/.udebs. :)

Heh.  You're right.

Here is a new version of the patch, fixing this problem.  I am not sure
that it is meaningful to require that this change be seconded, but out
of (possibly too much) respect for process, seeking seconds (and CCing
those who have already seconded in the hope they'll renew their
seconds):

diff --git a/policy/ch-source.rst b/policy/ch-source.rst
index dc80243..3c6c9d5 100644
--- a/policy/ch-source.rst
+++ b/policy/ch-source.rst
@@ -291,6 +291,20 @@ For packages in the main archive, no required targets may 
attempt
 network access, except, via the loopback interface, to services on the
 build host that have been started by the build.

+Required targets must not attempt to write outside of the unpacked
+source package tree.  There are two exceptions.  Firstly, the binary
+targets may write the binary packages to the parent directory of the
+unpacked source package tree.  Secondly, required targets may write to
+the directory specified by the ``TMPDIR`` environment variable (or
+``/tmp`` if that is not set), provided that files created in that
+directory are deleted before the target completes and are not reused
+by subsequent executions of the target.
+
+This restriction is intended to prevent source package builds creating
+and depending on state outside of themselves, thus affecting multiple
+independent rebuilds.  In particular, the required targets must not
+attempt to write into ``HOME``.
+
 The targets are as follows:

 ``build`` (required)

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#845715: Required targets must not write outside of the source package tree

[Niels Thykier]

On Sat, 03 Nov 2018 12:38:55 -0700 Sean Whitton
 wrote:
> control: tag -1 +patch
> 
> Hello,
> 
> I reformatted and wordsmithed josch's patch, second it myself, and am
> seeking further seconds.
> 
> Given that whole archive rebuilds with use sbuild and already catch
> packages that violate this requirement, making this change would not
> declare any packages buggy that would not already be considered buggy,
> so we can make it right away.
> 
> [...]
> index dc80243..c486e7c 100644
> --- a/policy/ch-source.rst
> +++ b/policy/ch-source.rst
> @@ -291,6 +291,16 @@ For packages in the main archive, no required targets 
> may attempt
>  network access, except, via the loopback interface, to services on the
>  build host that have been started by the build.
> 
> +Required targets must not attempt to write outside of the unpacked
> +source package tree. An exception to this rule is the use of
> +``TMPDIR`` (or ``/tmp`` if that is not set) which is permitted as long
> +as temporary files are deleted by the end of the target, and not
> +reused by subsequent execution of the target.  This restriction is
> +intended to prevent source package builds creating and depending on
> +state outside of themselves, thus affecting multiple independent
> +rebuilds.  In particular, the required targets must not attempt to
> +write into ``HOME``.
> +
> [...]

I suspect we are missing an exception allowing the binary targets to
write the produced binaries in the parent directory of the unpacked
source tree.
  Otherwise pretty much all packages violate the policy when they
generate the actual .debs/.udebs. :)

Thanks,
~Niels

Bug#845715: Required targets must not write outside of the source package tree

[Mattia Rizzolo]

Hi,

On Sat, Nov 03, 2018 at 12:38:55PM -0700, Sean Whitton wrote:
> Given that whole archive rebuilds with use sbuild and already catch
> packages that violate this requirement, making this change would not
> declare any packages buggy that would not already be considered buggy,
> so we can make it right away.

That's not entirely true, I can very easily imagine stuff trying to
write to $HOME but, if failing, trying elsewhere…



Anyway, seconded the below, with or without Russ' amend in
<87woptdiwa@hope.eyrie.org>.
Thank you!

> diff --git a/debian/changelog b/debian/changelog
> index 956f367..b90ea92 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -10,6 +10,11 @@ debian-policy (4.2.2.0) UNRELEASED; urgency=medium
>  Seconded: Holger Levsen 
>  Seconded: Russ Allbery 
>  Closes: #912581
> +  * Policy: Required targets must not write outside of the source package 
> tree
> +Wording: Johannes Schauer 
> +Seconded: Sean Whitton 
> +Seconded: ...
> +Closes: #845715
>* In a preexisting footnote, recommend passing -D to strip(1) when
>  stripping static libraries.
>  Thanks to Niels Thykier for the suggestion.
> diff --git a/policy/ch-source.rst b/policy/ch-source.rst
> index dc80243..c486e7c 100644
> --- a/policy/ch-source.rst
> +++ b/policy/ch-source.rst
> @@ -291,6 +291,16 @@ For packages in the main archive, no required targets 
> may attempt
>  network access, except, via the loopback interface, to services on the
>  build host that have been started by the build.
> 
> +Required targets must not attempt to write outside of the unpacked
> +source package tree. An exception to this rule is the use of
> +``TMPDIR`` (or ``/tmp`` if that is not set) which is permitted as long
> +as temporary files are deleted by the end of the target, and not
> +reused by subsequent execution of the target.  This restriction is
> +intended to prevent source package builds creating and depending on
> +state outside of themselves, thus affecting multiple independent
> +rebuilds.  In particular, the required targets must not attempt to
> +write into ``HOME``.
> +
>  The targets are as follows:
> 
>  ``build`` (required)
> diff --git a/policy/upgrading-checklist.rst b/policy/upgrading-checklist.rst
> index 899f7e8..70b31bd 100644
> --- a/policy/upgrading-checklist.rst
> +++ b/policy/upgrading-checklist.rst
> @@ -52,6 +52,10 @@ Unreleased.
>  copyright file, but it need not be if creating and maintaining a
>  copy of that information involves significant time and effort
> 
> +4.9
> +Required targets must not write outside of the unpacked source
> +package tree, except for TMPDIR (or /tmp if that is not set).
> +
>  10.1
>  Binaries should be stripped using
>  ``strip --strip-unneeded --remove-section=.comment 
> --remove-section=.note``

-- 
regards,
Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540  .''`.
more about me:  https://mapreri.org : :'  :
Launchpad user: https://launchpad.net/~mapreri  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-


signature.asc
Description: PGP signature

Bug#845715: Required targets must not write outside of the source package tree

[Bill Allombert]

On Sat, Nov 03, 2018 at 01:07:49PM -0700, Russ Allbery wrote:
> Sean Whitton  writes:
> 
> > I reformatted and wordsmithed josch's patch, second it myself, and am
> > seeking further seconds.
> 
> > Given that whole archive rebuilds with use sbuild and already catch
> > packages that violate this requirement, making this change would not
> > declare any packages buggy that would not already be considered buggy,
> > so we can make it right away.
> 
> Excellent!  This has been a long-standing issue, and it's great to finally
> get this into Policy.
> 
> One minor wording nit, seconded either way:
> 
> > +Required targets must not attempt to write outside of the unpacked
> > +source package tree. An exception to this rule is the use of
> > +``TMPDIR`` (or ``/tmp`` if that is not set) which is permitted as long
> > +as temporary files are deleted by the end of the target, and not
> > +reused by subsequent execution of the target.  This restriction is
> 
> How about:
> 
> As an exception, required targets may write to the directory specified
> by the ``TMPDIR`` environment variable (or ``/tmp`` if that is not
> set), provided that files created in that directory are deleted before
> the target completes and are not reused by subsequent executions of
> the target.
> 
> This explicitly states that this is an environment variable and makes it
> clear that it refers to a directory.

I may be wrong but I expect that a lot of packages targets leaves stray
files and directory in /tmp (that are created by mktemp but not removed)
but not reused, and also that some of them fail to honor TMPDIR.

Do we have some data about this ?

Cheers,
-- 
Bill. 

Imagine a large red swirl here. 

Bug#845715: Required targets must not write outside of the source package tree

[Sean Whitton]

control: tag -1 -patch +pending

Hello,

On Sat 03 Nov 2018 at 01:07PM -0700, Russ Allbery wrote:

> One minor wording nit, seconded either way:
>
>> +Required targets must not attempt to write outside of the unpacked
>> +source package tree. An exception to this rule is the use of
>> +``TMPDIR`` (or ``/tmp`` if that is not set) which is permitted as long
>> +as temporary files are deleted by the end of the target, and not
>> +reused by subsequent execution of the target.  This restriction is
>
> How about:
>
> As an exception, required targets may write to the directory specified
> by the ``TMPDIR`` environment variable (or ``/tmp`` if that is not
> set), provided that files created in that directory are deleted before
> the target completes and are not reused by subsequent executions of
> the target.

Good, thanks.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#845715: Required targets must not write outside of the source package tree

[Russ Allbery]

Sean Whitton  writes:

> I reformatted and wordsmithed josch's patch, second it myself, and am
> seeking further seconds.

> Given that whole archive rebuilds with use sbuild and already catch
> packages that violate this requirement, making this change would not
> declare any packages buggy that would not already be considered buggy,
> so we can make it right away.

Excellent!  This has been a long-standing issue, and it's great to finally
get this into Policy.

One minor wording nit, seconded either way:

> +Required targets must not attempt to write outside of the unpacked
> +source package tree. An exception to this rule is the use of
> +``TMPDIR`` (or ``/tmp`` if that is not set) which is permitted as long
> +as temporary files are deleted by the end of the target, and not
> +reused by subsequent execution of the target.  This restriction is

How about:

As an exception, required targets may write to the directory specified
by the ``TMPDIR`` environment variable (or ``/tmp`` if that is not
set), provided that files created in that directory are deleted before
the target completes and are not reused by subsequent executions of
the target.

This explicitly states that this is an environment variable and makes it
clear that it refers to a directory.

> +intended to prevent source package builds creating and depending on
> +state outside of themselves, thus affecting multiple independent
> +rebuilds.  In particular, the required targets must not attempt to
> +write into ``HOME``.

-- 
Russ Allbery (r...@debian.org)   

Bug#845715: Required targets must not write outside of the source package tree

[Sean Whitton]

control: tag -1 +patch

Hello,

I reformatted and wordsmithed josch's patch, second it myself, and am
seeking further seconds.

Given that whole archive rebuilds with use sbuild and already catch
packages that violate this requirement, making this change would not
declare any packages buggy that would not already be considered buggy,
so we can make it right away.

diff --git a/debian/changelog b/debian/changelog
index 956f367..b90ea92 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -10,6 +10,11 @@ debian-policy (4.2.2.0) UNRELEASED; urgency=medium
 Seconded: Holger Levsen 
 Seconded: Russ Allbery 
 Closes: #912581
+  * Policy: Required targets must not write outside of the source package tree
+Wording: Johannes Schauer 
+Seconded: Sean Whitton 
+Seconded: ...
+Closes: #845715
   * In a preexisting footnote, recommend passing -D to strip(1) when
 stripping static libraries.
 Thanks to Niels Thykier for the suggestion.
diff --git a/policy/ch-source.rst b/policy/ch-source.rst
index dc80243..c486e7c 100644
--- a/policy/ch-source.rst
+++ b/policy/ch-source.rst
@@ -291,6 +291,16 @@ For packages in the main archive, no required targets may 
attempt
 network access, except, via the loopback interface, to services on the
 build host that have been started by the build.

+Required targets must not attempt to write outside of the unpacked
+source package tree. An exception to this rule is the use of
+``TMPDIR`` (or ``/tmp`` if that is not set) which is permitted as long
+as temporary files are deleted by the end of the target, and not
+reused by subsequent execution of the target.  This restriction is
+intended to prevent source package builds creating and depending on
+state outside of themselves, thus affecting multiple independent
+rebuilds.  In particular, the required targets must not attempt to
+write into ``HOME``.
+
 The targets are as follows:

 ``build`` (required)
diff --git a/policy/upgrading-checklist.rst b/policy/upgrading-checklist.rst
index 899f7e8..70b31bd 100644
--- a/policy/upgrading-checklist.rst
+++ b/policy/upgrading-checklist.rst
@@ -52,6 +52,10 @@ Unreleased.
 copyright file, but it need not be if creating and maintaining a
 copy of that information involves significant time and effort

+4.9
+Required targets must not write outside of the unpacked source
+package tree, except for TMPDIR (or /tmp if that is not set).
+
 10.1
 Binaries should be stripped using
 ``strip --strip-unneeded --remove-section=.comment --remove-section=.note``

-- 
Sean Whitton


signature.asc
Description: PGP signature